Merge branch 'master' into abi

13 files changed, 63 insertions, 71 deletions

diff --git a/src/libstd/sys/cloudabi/stack_overflow.rs b/src/libstd/sys/cloudabi/stack_overflow.rs
index e97831b2c28..9339b143731 100644
--- a/src/libstd/sys/cloudabi/stack_overflow.rs
+++ b/src/libstd/sys/cloudabi/stack_overflow.rs
@@ -1,13 +1,5 @@
 #![cfg_attr(test, allow(dead_code))]
 
-pub struct Handler;
-
-impl Handler {
-    pub unsafe fn new() -> Handler {
-        Handler
-    }
-}
-
 pub unsafe fn init() {}
 
 pub unsafe fn cleanup() {}
diff --git a/src/libstd/sys/cloudabi/thread.rs b/src/libstd/sys/cloudabi/thread.rs
index 3afcae7ae75..a15dc8653e8 100644
--- a/src/libstd/sys/cloudabi/thread.rs
+++ b/src/libstd/sys/cloudabi/thread.rs
@@ -5,7 +5,6 @@ use crate::mem;
 use crate::ptr;
 use crate::sys::cloudabi::abi;
 use crate::sys::time::checked_dur2intervals;
-use crate::sys_common::thread::*;
 use crate::time::Duration;
 
 pub const DEFAULT_MIN_STACK_SIZE: usize = 2 * 1024 * 1024;
@@ -22,7 +21,7 @@ unsafe impl Sync for Thread {}
 impl Thread {
     // unsafe: see thread::Builder::spawn_unchecked for safety requirements
     pub unsafe fn new(stack: usize, p: Box<dyn FnOnce()>) -> io::Result<Thread> {
-        let p = box p;
+        let p = Box::into_raw(box 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);
@@ -30,19 +29,25 @@ impl Thread {
         let stack_size = cmp::max(stack, min_stack_size(&attr));
         assert_eq!(libc::pthread_attr_setstacksize(&mut attr, stack_size), 0);
 
-        let ret = libc::pthread_create(&mut native, &attr, thread_start, &*p as *const _ as *mut _);
+        let ret = libc::pthread_create(&mut native, &attr, 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);
 
         return if ret != 0 {
+            // The thread failed to start and as a result p was not consumed. Therefore, it is
+            // safe to reconstruct the box so that it gets deallocated.
+            drop(Box::from_raw(p));
             Err(io::Error::from_raw_os_error(ret))
         } else {
-            mem::forget(p); // ownership passed to pthread_create
             Ok(Thread { id: native })
         };
 
         extern "C" fn thread_start(main: *mut libc::c_void) -> *mut libc::c_void {
             unsafe {
-                start_thread(main as *mut u8);
+                // Let's run some code.
+                Box::from_raw(main as *mut Box<dyn FnOnce()>)();
             }
             ptr::null_mut()
         }
diff --git a/src/libstd/sys/hermit/stack_overflow.rs b/src/libstd/sys/hermit/stack_overflow.rs
index 65a1b17acce..121fe42011d 100644
--- a/src/libstd/sys/hermit/stack_overflow.rs
+++ b/src/libstd/sys/hermit/stack_overflow.rs
@@ -1,11 +1,3 @@
-pub struct Handler;
-
-impl Handler {
-    pub unsafe fn new() -> Handler {
-        Handler
-    }
-}
-
 #[inline]
 pub unsafe fn init() {}
 
diff --git a/src/libstd/sys/hermit/thread.rs b/src/libstd/sys/hermit/thread.rs
index 00d760d02e8..8a718a8915b 100644
--- a/src/libstd/sys/hermit/thread.rs
+++ b/src/libstd/sys/hermit/thread.rs
@@ -7,8 +7,6 @@ use crate::sys::hermit::abi;
 use crate::time::Duration;
 use core::u32;
 
-use crate::sys_common::thread::*;
-
 pub type Tid = abi::Tid;
 
 pub struct Thread {
@@ -26,7 +24,7 @@ impl Thread {
         p: Box<dyn FnOnce()>,
         core_id: isize,
     ) -> io::Result<Thread> {
-        let p = box p;
+        let p = Box::into_raw(box p);
         let mut tid: Tid = u32::MAX;
         let ret = abi::spawn(
             &mut tid as *mut Tid,
@@ -36,16 +34,19 @@ impl Thread {
             core_id,
         );
 
-        return if ret == 0 {
-            mem::forget(p); // ownership passed to pthread_create
-            Ok(Thread { tid: tid })
-        } else {
+        return if ret != 0 {
+            // The thread failed to start and as a result p was not consumed. Therefore, it is
+            // safe to reconstruct the box so that it gets deallocated.
+            drop(Box::from_raw(p));
             Err(io::Error::new(io::ErrorKind::Other, "Unable to create thread!"))
+        } else {
+            Ok(Thread { tid: tid })
         };
 
         extern "C" fn thread_start(main: usize) {
             unsafe {
-                start_thread(main as *mut u8);
+                // Finally, let's run some code.
+                Box::from_raw(main as *mut Box<dyn FnOnce()>)();
             }
         }
     }
diff --git a/src/libstd/sys/sgx/stack_overflow.rs b/src/libstd/sys/sgx/stack_overflow.rs
index a2d13d11849..b96652a8330 100644
--- a/src/libstd/sys/sgx/stack_overflow.rs
+++ b/src/libstd/sys/sgx/stack_overflow.rs
@@ -1,11 +1,3 @@
-pub struct Handler;
-
-impl Handler {
-    pub unsafe fn new() -> Handler {
-        Handler
-    }
-}
-
 #[cfg_attr(test, allow(dead_code))]
 pub unsafe fn init() {}
 
diff --git a/src/libstd/sys/unix/process/process_unix.rs b/src/libstd/sys/unix/process/process_unix.rs
index 07d0fbf61fe..f389c60615f 100644
--- a/src/libstd/sys/unix/process/process_unix.rs
+++ b/src/libstd/sys/unix/process/process_unix.rs
@@ -72,7 +72,7 @@ impl Command {
             }
         };
 
-        let mut p = Process { pid: pid, status: None };
+        let mut p = Process { pid, status: None };
         drop(output);
         let mut bytes = [0; 8];
 
diff --git a/src/libstd/sys/unix/thread.rs b/src/libstd/sys/unix/thread.rs
index 674d4c71138..aab5a92a7ad 100644
--- a/src/libstd/sys/unix/thread.rs
+++ b/src/libstd/sys/unix/thread.rs
@@ -3,11 +3,9 @@ use crate::ffi::CStr;
 use crate::io;
 use crate::mem;
 use crate::ptr;
-use crate::sys::os;
+use crate::sys::{os, stack_overflow};
 use crate::time::Duration;
 
-use crate::sys_common::thread::*;
-
 #[cfg(not(target_os = "l4re"))]
 pub const DEFAULT_MIN_STACK_SIZE: usize = 2 * 1024 * 1024;
 #[cfg(target_os = "l4re")]
@@ -43,7 +41,7 @@ unsafe fn pthread_attr_setstacksize(
 impl Thread {
     // unsafe: see thread::Builder::spawn_unchecked for safety requirements
     pub unsafe fn new(stack: usize, p: Box<dyn FnOnce()>) -> io::Result<Thread> {
-        let p = box p;
+        let p = Box::into_raw(box 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);
@@ -65,19 +63,28 @@ impl Thread {
             }
         };
 
-        let ret = libc::pthread_create(&mut native, &attr, thread_start, &*p as *const _ as *mut _);
+        let ret = libc::pthread_create(&mut native, &attr, 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);
 
         return if ret != 0 {
+            // The thread failed to start and as a result p was not consumed. Therefore, it is
+            // safe to reconstruct the box so that it gets deallocated.
+            drop(Box::from_raw(p));
             Err(io::Error::from_raw_os_error(ret))
         } else {
-            mem::forget(p); // ownership passed to pthread_create
             Ok(Thread { id: native })
         };
 
         extern "C" fn thread_start(main: *mut libc::c_void) -> *mut libc::c_void {
             unsafe {
-                start_thread(main as *mut u8);
+                // Next, set up our stack overflow handler which may get triggered if we run
+                // out of stack.
+                let _handler = stack_overflow::Handler::new();
+                // Finally, let's run some code.
+                Box::from_raw(main as *mut Box<dyn FnOnce()>)();
             }
             ptr::null_mut()
         }
diff --git a/src/libstd/sys/vxworks/thread.rs b/src/libstd/sys/vxworks/thread.rs
index e0d104b5f3e..4d0196e4b4d 100644
--- a/src/libstd/sys/vxworks/thread.rs
+++ b/src/libstd/sys/vxworks/thread.rs
@@ -3,11 +3,9 @@ use crate::ffi::CStr;
 use crate::io;
 use crate::mem;
 use crate::ptr;
-use crate::sys::os;
+use crate::sys::{os, stack_overflow};
 use crate::time::Duration;
 
-use crate::sys_common::thread::*;
-
 pub const DEFAULT_MIN_STACK_SIZE: usize = 0x40000; // 256K
 
 pub struct Thread {
@@ -31,7 +29,7 @@ unsafe fn pthread_attr_setstacksize(
 impl Thread {
     // unsafe: see thread::Builder::spawn_unchecked for safety requirements
     pub unsafe fn new(stack: usize, p: Box<dyn FnOnce()>) -> io::Result<Thread> {
-        let p = box p;
+        let p = Box::into_raw(box 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);
@@ -53,19 +51,28 @@ impl Thread {
             }
         };
 
-        let ret = libc::pthread_create(&mut native, &attr, thread_start, &*p as *const _ as *mut _);
+        let ret = libc::pthread_create(&mut native, &attr, 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);
 
         return if ret != 0 {
+            // The thread failed to start and as a result p was not consumed. Therefore, it is
+            // safe to reconstruct the box so that it gets deallocated.
+            drop(Box::from_raw(p));
             Err(io::Error::from_raw_os_error(ret))
         } else {
-            mem::forget(p); // ownership passed to pthread_create
             Ok(Thread { id: native })
         };
 
         extern "C" fn thread_start(main: *mut libc::c_void) -> *mut libc::c_void {
             unsafe {
-                start_thread(main as *mut u8);
+                // Next, set up our stack overflow handler which may get triggered if we run
+                // out of stack.
+                let _handler = stack_overflow::Handler::new();
+                // Finally, let's run some code.
+                Box::from_raw(main as *mut Box<dyn FnOnce()>)();
             }
             ptr::null_mut()
         }
diff --git a/src/libstd/sys/wasi/alloc.rs b/src/libstd/sys/wasi/alloc.rs
index e9760d050e1..bc614162784 100644
--- a/src/libstd/sys/wasi/alloc.rs
+++ b/src/libstd/sys/wasi/alloc.rs
@@ -10,7 +10,7 @@ unsafe impl GlobalAlloc for System {
         if layout.align() <= MIN_ALIGN && layout.align() <= layout.size() {
             libc::malloc(layout.size()) as *mut u8
         } else {
-            libc::aligned_alloc(layout.size(), layout.align()) as *mut u8
+            libc::aligned_alloc(layout.align(), layout.size()) as *mut u8
         }
     }
 
diff --git a/src/libstd/sys/wasm/stack_overflow.rs b/src/libstd/sys/wasm/stack_overflow.rs
index cbf62b6e5b7..32555394cd5 100644
--- a/src/libstd/sys/wasm/stack_overflow.rs
+++ b/src/libstd/sys/wasm/stack_overflow.rs
@@ -1,11 +1,3 @@
-pub struct Handler;
-
-impl Handler {
-    pub unsafe fn new() -> Handler {
-        Handler
-    }
-}
-
 pub unsafe fn init() {}
 
 pub unsafe fn cleanup() {}
diff --git a/src/libstd/sys/windows/handle.rs b/src/libstd/sys/windows/handle.rs
index f2ad057b6b6..d00381792e3 100644
--- a/src/libstd/sys/windows/handle.rs
+++ b/src/libstd/sys/windows/handle.rs
@@ -115,8 +115,7 @@ impl RawHandle {
     ) -> io::Result<Option<usize>> {
         let len = cmp::min(buf.len(), <c::DWORD>::max_value() as usize) as c::DWORD;
         let mut amt = 0;
-        let res =
-            cvt({ c::ReadFile(self.0, buf.as_ptr() as c::LPVOID, len, &mut amt, overlapped) });
+        let res = cvt(c::ReadFile(self.0, buf.as_ptr() as c::LPVOID, len, &mut amt, overlapped));
         match res {
             Ok(_) => Ok(Some(amt as usize)),
             Err(e) => {
@@ -139,7 +138,7 @@ impl RawHandle {
         unsafe {
             let mut bytes = 0;
             let wait = if wait { c::TRUE } else { c::FALSE };
-            let res = cvt({ c::GetOverlappedResult(self.raw(), overlapped, &mut bytes, wait) });
+            let res = cvt(c::GetOverlappedResult(self.raw(), overlapped, &mut bytes, wait));
             match res {
                 Ok(_) => Ok(bytes as usize),
                 Err(e) => {
diff --git a/src/libstd/sys/windows/os.rs b/src/libstd/sys/windows/os.rs
index cc4ae405906..a0da2498bb7 100644
--- a/src/libstd/sys/windows/os.rs
+++ b/src/libstd/sys/windows/os.rs
@@ -94,7 +94,7 @@ impl Iterator for Env {
                 if *self.cur == 0 {
                     return None;
                 }
-                let p = &*self.cur as *const u16;
+                let p = self.cur as *const u16;
                 let mut len = 0;
                 while *p.offset(len) != 0 {
                     len += 1;
diff --git a/src/libstd/sys/windows/thread.rs b/src/libstd/sys/windows/thread.rs
index c828243a59b..38839ea5e90 100644
--- a/src/libstd/sys/windows/thread.rs
+++ b/src/libstd/sys/windows/thread.rs
@@ -1,10 +1,9 @@
 use crate::ffi::CStr;
 use crate::io;
-use crate::mem;
 use crate::ptr;
 use crate::sys::c;
 use crate::sys::handle::Handle;
-use crate::sys_common::thread::*;
+use crate::sys::stack_overflow;
 use crate::time::Duration;
 
 use libc::c_void;
@@ -20,7 +19,7 @@ pub struct Thread {
 impl Thread {
     // unsafe: see thread::Builder::spawn_unchecked for safety requirements
     pub unsafe fn new(stack: usize, p: Box<dyn FnOnce()>) -> io::Result<Thread> {
-        let p = box p;
+        let p = Box::into_raw(box p);
 
         // FIXME On UNIX, we guard against stack sizes that are too small but
         // that's because pthreads enforces that stacks are at least
@@ -34,21 +33,27 @@ impl Thread {
             ptr::null_mut(),
             stack_size,
             thread_start,
-            &*p as *const _ as *mut _,
+            p as *mut _,
             c::STACK_SIZE_PARAM_IS_A_RESERVATION,
             ptr::null_mut(),
         );
 
         return if ret as usize == 0 {
+            // The thread failed to start and as a result p was not consumed. Therefore, it is
+            // safe to reconstruct the box so that it gets deallocated.
+            drop(Box::from_raw(p));
             Err(io::Error::last_os_error())
         } else {
-            mem::forget(p); // ownership passed to CreateThread
             Ok(Thread { handle: Handle::new(ret) })
         };
 
         extern "system" fn thread_start(main: *mut c_void) -> c::DWORD {
             unsafe {
-                start_thread(main as *mut u8);
+                // Next, set up our stack overflow handler which may get triggered if we run
+                // out of stack.
+                let _handler = stack_overflow::Handler::new();
+                // Finally, let's run some code.
+                Box::from_raw(main as *mut Box<dyn FnOnce()>)();
             }
             0
         }