From 7e7f5d385694ca345e0b47d8c59c7c966ee60a30 Mon Sep 17 00:00:00 2001 From: xizheyin Date: Tue, 11 Feb 2025 01:01:00 +0800 Subject: [PATCH 1/4] Replace mem::zeroed with mem::MaybeUninit::uninit for large struct in unix Signed-off-by: xizheyin --- library/std/src/sys/net/connection/socket.rs | 9 +++++--- .../std/src/sys/net/connection/socket/unix.rs | 7 ++++-- .../std/src/sys/pal/unix/stack_overflow.rs | 23 ++++++++++++++----- library/std/src/sys/pal/unix/thread.rs | 5 ++-- 4 files changed, 31 insertions(+), 13 deletions(-) diff --git a/library/std/src/sys/net/connection/socket.rs b/library/std/src/sys/net/connection/socket.rs index b4f0a7836803e..ddd74b426158d 100644 --- a/library/std/src/sys/net/connection/socket.rs +++ b/library/std/src/sys/net/connection/socket.rs @@ -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 = 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)) } diff --git a/library/std/src/sys/net/connection/socket/unix.rs b/library/std/src/sys/net/connection/socket/unix.rs index 34ab26bc117af..29fb47ddca3b9 100644 --- a/library/std/src/sys/net/connection/socket/unix.rs +++ b/library/std/src/sys/net/connection/socket/unix.rs @@ -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 = mem::MaybeUninit::uninit(); let mut addrlen = mem::size_of_val(&storage) as libc::socklen_t; let n = cvt(unsafe { @@ -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)> { diff --git a/library/std/src/sys/pal/unix/stack_overflow.rs b/library/std/src/sys/pal/unix/stack_overflow.rs index db5c6bd3a1c32..1ccf2011ea162 100644 --- a/library/std/src/sys/pal/unix/stack_overflow.rs +++ b/library/std/src/sys/pal/unix/stack_overflow.rs @@ -319,9 +319,14 @@ 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(); - #[cfg(target_os = "freebsd")] - assert_eq!(libc::pthread_attr_init(&mut attr), 0); + let attr: mem::MaybeUninit = if cfg!(target_os = "freebsd") { + let mut attr = mem::MaybeUninit::uninit(); + assert_eq!(libc::pthread_attr_init((&raw mut attr) as *mut _), 0); + attr + } else { + mem::MaybeUninit::zeroed() + }; + let mut attr = unsafe { attr.assume_init() }; #[cfg(target_os = "freebsd")] let e = libc::pthread_attr_get_np(libc::pthread_self(), &mut attr); #[cfg(not(target_os = "freebsd"))] @@ -509,9 +514,15 @@ mod imp { // FIXME: I am probably not unsafe. unsafe fn current_guard() -> Option> { let mut ret = None; - let mut attr: libc::pthread_attr_t = crate::mem::zeroed(); - #[cfg(target_os = "freebsd")] - assert_eq!(libc::pthread_attr_init(&mut attr), 0); + let attr: mem::MaybeUninit = if cfg!(target_os = "freebsd") { + let mut attr = mem::MaybeUninit::uninit(); + assert_eq!(libc::pthread_attr_init((&raw mut attr) as *mut _), 0); + attr + } else { + mem::MaybeUninit::zeroed() + }; + + let mut attr = unsafe { attr.assume_init() }; #[cfg(target_os = "freebsd")] let e = libc::pthread_attr_get_np(libc::pthread_self(), &mut attr); #[cfg(not(target_os = "freebsd"))] diff --git a/library/std/src/sys/pal/unix/thread.rs b/library/std/src/sys/pal/unix/thread.rs index 479021af040a7..c339970c5d376 100644 --- a/library/std/src/sys/pal/unix/thread.rs +++ b/library/std/src/sys/pal/unix/thread.rs @@ -49,8 +49,9 @@ impl Thread { pub unsafe fn new(stack: usize, p: Box) -> io::Result { 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 = mem::MaybeUninit::uninit(); + assert_eq!(libc::pthread_attr_init((&raw mut attr) as *mut _), 0); + let mut attr: libc::pthread_attr_t = unsafe { attr.assume_init() }; #[cfg(target_os = "espidf")] if stack > 0 { From c1ecdf112401cc03f5e0ed364892e5da04d3bf41 Mon Sep 17 00:00:00 2001 From: xizheyin Date: Tue, 11 Feb 2025 20:38:25 +0800 Subject: [PATCH 2/4] Consistently using as_mut_ptr() and as_ptr() in thread Signed-off-by: xizheyin --- library/std/src/sys/pal/unix/thread.rs | 19 +++++++++++-------- 1 file changed, 11 insertions(+), 8 deletions(-) diff --git a/library/std/src/sys/pal/unix/thread.rs b/library/std/src/sys/pal/unix/thread.rs index c339970c5d376..d476f6600bf47 100644 --- a/library/std/src/sys/pal/unix/thread.rs +++ b/library/std/src/sys/pal/unix/thread.rs @@ -50,24 +50,27 @@ impl Thread { let p = Box::into_raw(Box::new(p)); let mut native: libc::pthread_t = mem::zeroed(); let mut attr: mem::MaybeUninit = mem::MaybeUninit::uninit(); - assert_eq!(libc::pthread_attr_init((&raw mut attr) as *mut _), 0); - let mut attr: libc::pthread_attr_t = unsafe { attr.assume_init() }; + assert_eq!(libc::pthread_attr_init(attr.as_mut_ptr()), 0); + //let mut attr: libc::pthread_attr_t = unsafe { attr.assume_init() }; #[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); @@ -78,16 +81,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 From 604364fcf44e0843e4ea9818daaf97418da233d5 Mon Sep 17 00:00:00 2001 From: xizheyin Date: Wed, 19 Feb 2025 13:20:03 +0800 Subject: [PATCH 3/4] remove assume_init in stack_overflow Signed-off-by: xizheyin --- library/std/src/sys/pal/unix/stack_overflow.rs | 15 +++++++++------ library/std/src/sys/pal/unix/thread.rs | 1 - 2 files changed, 9 insertions(+), 7 deletions(-) diff --git a/library/std/src/sys/pal/unix/stack_overflow.rs b/library/std/src/sys/pal/unix/stack_overflow.rs index 1ccf2011ea162..0550223eb9ac3 100644 --- a/library/std/src/sys/pal/unix/stack_overflow.rs +++ b/library/std/src/sys/pal/unix/stack_overflow.rs @@ -319,26 +319,29 @@ mod imp { ))] unsafe fn get_stack_start() -> Option<*mut libc::c_void> { let mut ret = None; - let attr: mem::MaybeUninit = if cfg!(target_os = "freebsd") { + let mut attr: mem::MaybeUninit = if cfg!(target_os = "freebsd") { let mut attr = mem::MaybeUninit::uninit(); assert_eq!(libc::pthread_attr_init((&raw mut attr) as *mut _), 0); attr } else { mem::MaybeUninit::zeroed() }; - let mut attr = unsafe { attr.assume_init() }; + #[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 } diff --git a/library/std/src/sys/pal/unix/thread.rs b/library/std/src/sys/pal/unix/thread.rs index d476f6600bf47..e23d3d6d647ae 100644 --- a/library/std/src/sys/pal/unix/thread.rs +++ b/library/std/src/sys/pal/unix/thread.rs @@ -51,7 +51,6 @@ impl Thread { let mut native: libc::pthread_t = mem::zeroed(); let mut attr: mem::MaybeUninit = mem::MaybeUninit::uninit(); assert_eq!(libc::pthread_attr_init(attr.as_mut_ptr()), 0); - //let mut attr: libc::pthread_attr_t = unsafe { attr.assume_init() }; #[cfg(target_os = "espidf")] if stack > 0 { From 70f11ee0c063fa85dff72306c2ec29532b8cabeb Mon Sep 17 00:00:00 2001 From: xizheyin Date: Wed, 19 Feb 2025 20:16:28 +0800 Subject: [PATCH 4/4] fix by comments Signed-off-by: xizheyin --- .../std/src/sys/pal/unix/stack_overflow.rs | 38 +++++++++---------- 1 file changed, 17 insertions(+), 21 deletions(-) diff --git a/library/std/src/sys/pal/unix/stack_overflow.rs b/library/std/src/sys/pal/unix/stack_overflow.rs index 0550223eb9ac3..43ece63457fe6 100644 --- a/library/std/src/sys/pal/unix/stack_overflow.rs +++ b/library/std/src/sys/pal/unix/stack_overflow.rs @@ -319,14 +319,12 @@ mod imp { ))] unsafe fn get_stack_start() -> Option<*mut libc::c_void> { let mut ret = None; - let mut attr: mem::MaybeUninit = if cfg!(target_os = "freebsd") { - let mut attr = mem::MaybeUninit::uninit(); - assert_eq!(libc::pthread_attr_init((&raw mut attr) as *mut _), 0); - attr - } else { - mem::MaybeUninit::zeroed() - }; - + let mut attr: mem::MaybeUninit = mem::MaybeUninit::uninit(); + if !cfg!(target_os = "freebsd") { + attr = mem::MaybeUninit::zeroed(); + } + #[cfg(target_os = "freebsd")] + 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(), attr.as_mut_ptr()); #[cfg(not(target_os = "freebsd"))] @@ -517,22 +515,20 @@ mod imp { // FIXME: I am probably not unsafe. unsafe fn current_guard() -> Option> { let mut ret = None; - let attr: mem::MaybeUninit = if cfg!(target_os = "freebsd") { - let mut attr = mem::MaybeUninit::uninit(); - assert_eq!(libc::pthread_attr_init((&raw mut attr) as *mut _), 0); - attr - } else { - mem::MaybeUninit::zeroed() - }; - let mut attr = unsafe { attr.assume_init() }; + let mut attr: mem::MaybeUninit = mem::MaybeUninit::uninit(); + if !cfg!(target_os = "freebsd") { + attr = mem::MaybeUninit::zeroed(); + } + #[cfg(target_os = "freebsd")] + 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 @@ -545,7 +541,7 @@ mod imp { } let mut stackptr = crate::ptr::null_mut::(); 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")) { @@ -566,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 }