Skip to content

Commit

Permalink
Unrolled build for rust-lang#136826
Browse files Browse the repository at this point in the history
Rollup merge of rust-lang#136826 - xizheyin:issue-136737, r=thomcc

Replace mem::zeroed with mem::MaybeUninit::uninit for large struct in Unix

As discussion in rust-lang#136737.

- Replace `mem::zeroed()` with `MaybeUninit::uninit()` for `sockaddr_storage` in `accept()` and `recvfrom()` since these functions fill in the address structure
- Replace `mem::zeroed()` with `MaybeUninit::uninit()` for `pthread_attr_t` in thread-related functions since `pthread_attr_init()` initializes the structure
- Add references to man pages to document this behavior
  • Loading branch information
rust-timer authored Feb 23, 2025
2 parents bb2cc59 + 70f11ee commit cbf6492
Show file tree
Hide file tree
Showing 4 changed files with 45 additions and 26 deletions.
9 changes: 6 additions & 3 deletions library/std/src/sys/net/connection/socket.rs
Original file line number Diff line number Diff line change
Expand Up @@ -557,10 +557,13 @@ impl TcpListener {
}

pub fn accept(&self) -> io::Result<(TcpStream, SocketAddr)> {
let mut storage: c::sockaddr_storage = unsafe { mem::zeroed() };
// The `accept` function will fill in the storage with the address,
// so we don't need to zero it here.
// reference: https://linux.die.net/man/2/accept4
let mut storage: mem::MaybeUninit<c::sockaddr_storage> = mem::MaybeUninit::uninit();
let mut len = mem::size_of_val(&storage) as c::socklen_t;
let sock = self.inner.accept((&raw mut storage) as *mut _, &mut len)?;
let addr = unsafe { socket_addr_from_c(&storage, len as usize)? };
let sock = self.inner.accept(storage.as_mut_ptr() as *mut _, &mut len)?;
let addr = unsafe { socket_addr_from_c(storage.as_ptr(), len as usize)? };
Ok((TcpStream { inner: sock }, addr))
}

Expand Down
7 changes: 5 additions & 2 deletions library/std/src/sys/net/connection/socket/unix.rs
Original file line number Diff line number Diff line change
Expand Up @@ -322,7 +322,10 @@ impl Socket {
buf: &mut [u8],
flags: c_int,
) -> io::Result<(usize, SocketAddr)> {
let mut storage: libc::sockaddr_storage = unsafe { mem::zeroed() };
// The `recvfrom` function will fill in the storage with the address,
// so we don't need to zero it here.
// reference: https://linux.die.net/man/2/recvfrom
let mut storage: mem::MaybeUninit<libc::sockaddr_storage> = mem::MaybeUninit::uninit();
let mut addrlen = mem::size_of_val(&storage) as libc::socklen_t;

let n = cvt(unsafe {
Expand All @@ -335,7 +338,7 @@ impl Socket {
&mut addrlen,
)
})?;
Ok((n as usize, unsafe { socket_addr_from_c(&storage, addrlen as usize)? }))
Ok((n as usize, unsafe { socket_addr_from_c(storage.as_ptr(), addrlen as usize)? }))
}

pub fn recv_from(&self, buf: &mut [u8]) -> io::Result<(usize, SocketAddr)> {
Expand Down
36 changes: 23 additions & 13 deletions library/std/src/sys/pal/unix/stack_overflow.rs
Original file line number Diff line number Diff line change
Expand Up @@ -319,21 +319,27 @@ mod imp {
))]
unsafe fn get_stack_start() -> Option<*mut libc::c_void> {
let mut ret = None;
let mut attr: libc::pthread_attr_t = crate::mem::zeroed();
let mut attr: mem::MaybeUninit<libc::pthread_attr_t> = mem::MaybeUninit::uninit();
if !cfg!(target_os = "freebsd") {
attr = mem::MaybeUninit::zeroed();
}
#[cfg(target_os = "freebsd")]
assert_eq!(libc::pthread_attr_init(&mut attr), 0);
assert_eq!(libc::pthread_attr_init(attr.as_mut_ptr()), 0);
#[cfg(target_os = "freebsd")]
let e = libc::pthread_attr_get_np(libc::pthread_self(), &mut attr);
let e = libc::pthread_attr_get_np(libc::pthread_self(), attr.as_mut_ptr());
#[cfg(not(target_os = "freebsd"))]
let e = libc::pthread_getattr_np(libc::pthread_self(), &mut attr);
let e = libc::pthread_getattr_np(libc::pthread_self(), attr.as_mut_ptr());
if e == 0 {
let mut stackaddr = crate::ptr::null_mut();
let mut stacksize = 0;
assert_eq!(libc::pthread_attr_getstack(&attr, &mut stackaddr, &mut stacksize), 0);
assert_eq!(
libc::pthread_attr_getstack(attr.as_ptr(), &mut stackaddr, &mut stacksize),
0
);
ret = Some(stackaddr);
}
if e == 0 || cfg!(target_os = "freebsd") {
assert_eq!(libc::pthread_attr_destroy(&mut attr), 0);
assert_eq!(libc::pthread_attr_destroy(attr.as_mut_ptr()), 0);
}
ret
}
Expand Down Expand Up @@ -509,16 +515,20 @@ mod imp {
// FIXME: I am probably not unsafe.
unsafe fn current_guard() -> Option<Range<usize>> {
let mut ret = None;
let mut attr: libc::pthread_attr_t = crate::mem::zeroed();

let mut attr: mem::MaybeUninit<libc::pthread_attr_t> = mem::MaybeUninit::uninit();
if !cfg!(target_os = "freebsd") {
attr = mem::MaybeUninit::zeroed();
}
#[cfg(target_os = "freebsd")]
assert_eq!(libc::pthread_attr_init(&mut attr), 0);
assert_eq!(libc::pthread_attr_init(attr.as_mut_ptr()), 0);
#[cfg(target_os = "freebsd")]
let e = libc::pthread_attr_get_np(libc::pthread_self(), &mut attr);
let e = libc::pthread_attr_get_np(libc::pthread_self(), attr.as_mut_ptr());
#[cfg(not(target_os = "freebsd"))]
let e = libc::pthread_getattr_np(libc::pthread_self(), &mut attr);
let e = libc::pthread_getattr_np(libc::pthread_self(), attr.as_mut_ptr());
if e == 0 {
let mut guardsize = 0;
assert_eq!(libc::pthread_attr_getguardsize(&attr, &mut guardsize), 0);
assert_eq!(libc::pthread_attr_getguardsize(attr.as_ptr(), &mut guardsize), 0);
if guardsize == 0 {
if cfg!(all(target_os = "linux", target_env = "musl")) {
// musl versions before 1.1.19 always reported guard
Expand All @@ -531,7 +541,7 @@ mod imp {
}
let mut stackptr = crate::ptr::null_mut::<libc::c_void>();
let mut size = 0;
assert_eq!(libc::pthread_attr_getstack(&attr, &mut stackptr, &mut size), 0);
assert_eq!(libc::pthread_attr_getstack(attr.as_ptr(), &mut stackptr, &mut size), 0);

let stackaddr = stackptr.addr();
ret = if cfg!(any(target_os = "freebsd", target_os = "netbsd", target_os = "hurd")) {
Expand All @@ -552,7 +562,7 @@ mod imp {
};
}
if e == 0 || cfg!(target_os = "freebsd") {
assert_eq!(libc::pthread_attr_destroy(&mut attr), 0);
assert_eq!(libc::pthread_attr_destroy(attr.as_mut_ptr()), 0);
}
ret
}
Expand Down
19 changes: 11 additions & 8 deletions library/std/src/sys/pal/unix/thread.rs
Original file line number Diff line number Diff line change
Expand Up @@ -49,24 +49,27 @@ impl Thread {
pub unsafe fn new(stack: usize, p: Box<dyn FnOnce()>) -> io::Result<Thread> {
let p = Box::into_raw(Box::new(p));
let mut native: libc::pthread_t = mem::zeroed();
let mut attr: libc::pthread_attr_t = mem::zeroed();
assert_eq!(libc::pthread_attr_init(&mut attr), 0);
let mut attr: mem::MaybeUninit<libc::pthread_attr_t> = mem::MaybeUninit::uninit();
assert_eq!(libc::pthread_attr_init(attr.as_mut_ptr()), 0);

#[cfg(target_os = "espidf")]
if stack > 0 {
// Only set the stack if a non-zero value is passed
// 0 is used as an indication that the default stack size configured in the ESP-IDF menuconfig system should be used
assert_eq!(
libc::pthread_attr_setstacksize(&mut attr, cmp::max(stack, min_stack_size(&attr))),
libc::pthread_attr_setstacksize(
attr.as_mut_ptr(),
cmp::max(stack, min_stack_size(&attr))
),
0
);
}

#[cfg(not(target_os = "espidf"))]
{
let stack_size = cmp::max(stack, min_stack_size(&attr));
let stack_size = cmp::max(stack, min_stack_size(attr.as_ptr()));

match libc::pthread_attr_setstacksize(&mut attr, stack_size) {
match libc::pthread_attr_setstacksize(attr.as_mut_ptr(), stack_size) {
0 => {}
n => {
assert_eq!(n, libc::EINVAL);
Expand All @@ -77,16 +80,16 @@ impl Thread {
let page_size = os::page_size();
let stack_size =
(stack_size + page_size - 1) & (-(page_size as isize - 1) as usize - 1);
assert_eq!(libc::pthread_attr_setstacksize(&mut attr, stack_size), 0);
assert_eq!(libc::pthread_attr_setstacksize(attr.as_mut_ptr(), stack_size), 0);
}
};
}

let ret = libc::pthread_create(&mut native, &attr, thread_start, p as *mut _);
let ret = libc::pthread_create(&mut native, attr.as_ptr(), thread_start, p as *mut _);
// Note: if the thread creation fails and this assert fails, then p will
// be leaked. However, an alternative design could cause double-free
// which is clearly worse.
assert_eq!(libc::pthread_attr_destroy(&mut attr), 0);
assert_eq!(libc::pthread_attr_destroy(attr.as_mut_ptr()), 0);

return if ret != 0 {
// The thread failed to start and as a result p was not consumed. Therefore, it is
Expand Down

0 comments on commit cbf6492

Please sign in to comment.