From e85c9227c2e913b71f0d7b6cc2322d7897f28554 Mon Sep 17 00:00:00 2001 From: Tatsuyuki Ishi Date: Tue, 27 Feb 2018 16:51:12 +0900 Subject: [PATCH 01/10] rustc_driver: get rid of extra thread on Unix --- src/librustc_driver/lib.rs | 51 +++++++++++++++++++++++----- src/libstd/sys_common/thread_info.rs | 4 +++ src/libstd/thread/mod.rs | 8 +++++ 3 files changed, 55 insertions(+), 8 deletions(-) diff --git a/src/librustc_driver/lib.rs b/src/librustc_driver/lib.rs index f1f3a0519bbcb..8605764497a16 100644 --- a/src/librustc_driver/lib.rs +++ b/src/librustc_driver/lib.rs @@ -24,6 +24,7 @@ #![feature(quote)] #![feature(rustc_diagnostic_macros)] #![feature(set_stdio)] +#![feature(rustc_stack_internals)] extern crate arena; extern crate getopts; @@ -1461,16 +1462,50 @@ pub fn in_rustc_thread(f: F) -> Result> // Temporarily have stack size set to 16MB to deal with nom-using crates failing const STACK_SIZE: usize = 16 * 1024 * 1024; // 16MB - let mut cfg = thread::Builder::new().name("rustc".to_string()); + #[cfg(unix)] + let spawn_thread = unsafe { + // Fetch the current resource limits + let mut rlim = libc::rlimit { + rlim_cur: 0, + rlim_max: 0, + }; + if libc::getrlimit(libc::RLIMIT_STACK, &mut rlim) != 0 { + let err = io::Error::last_os_error(); + error!("in_rustc_thread: error calling getrlimit: {}", err); + true + } else if rlim.rlim_max < STACK_SIZE as libc::rlim_t { + true + } else { + rlim.rlim_cur = STACK_SIZE as libc::rlim_t; + if libc::setrlimit(libc::RLIMIT_STACK, &mut rlim) != 0 { + let err = io::Error::last_os_error(); + error!("in_rustc_thread: error calling setrlimit: {}", err); + true + } else { + std::thread::update_stack_guard(); + false + } + } + }; - // FIXME: Hacks on hacks. If the env is trying to override the stack size - // then *don't* set it explicitly. - if env::var_os("RUST_MIN_STACK").is_none() { - cfg = cfg.stack_size(STACK_SIZE); - } + #[cfg(not(unix))] + let spawn_thread = true; + + // The or condition is added from backward compatibility. + if spawn_thread || env::var_os("RUST_MIN_STACK").is_some() { + let mut cfg = thread::Builder::new().name("rustc".to_string()); + + // FIXME: Hacks on hacks. If the env is trying to override the stack size + // then *don't* set it explicitly. + if env::var_os("RUST_MIN_STACK").is_none() { + cfg = cfg.stack_size(STACK_SIZE); + } - let thread = cfg.spawn(f); - thread.unwrap().join() + let thread = cfg.spawn(f); + thread.unwrap().join() + } else { + Ok(f()) + } } /// Get a list of extra command-line flags provided by the user, as strings. diff --git a/src/libstd/sys_common/thread_info.rs b/src/libstd/sys_common/thread_info.rs index 6a2b6742367a5..d75cbded7347b 100644 --- a/src/libstd/sys_common/thread_info.rs +++ b/src/libstd/sys_common/thread_info.rs @@ -50,3 +50,7 @@ pub fn set(stack_guard: Option, thread: Thread) { thread, })); } + +pub fn reset_guard(stack_guard: Option) { + THREAD_INFO.with(move |c| c.borrow_mut().as_mut().unwrap().stack_guard = stack_guard); +} diff --git a/src/libstd/thread/mod.rs b/src/libstd/thread/mod.rs index 71aee673cfe3e..b686ddc205ea7 100644 --- a/src/libstd/thread/mod.rs +++ b/src/libstd/thread/mod.rs @@ -208,6 +208,14 @@ pub use self::local::{LocalKey, AccessError}; #[unstable(feature = "libstd_thread_internals", issue = "0")] #[doc(hidden)] pub use self::local::os::Key as __OsLocalKeyInner; +/// Function used for resetting the main stack guard address after setrlimit(). +/// This is POSIX specific and unlikely to be directly stabilized. +#[unstable(feature = "rustc_stack_internals", issue = "0")] +pub unsafe fn update_stack_guard() { + let main_guard = imp::guard::init(); + thread_info::reset_guard(main_guard); +} + //////////////////////////////////////////////////////////////////////////////// // Builder //////////////////////////////////////////////////////////////////////////////// From 1bb89f1b3cf1e4b5fa83391872136251c0030c1e Mon Sep 17 00:00:00 2001 From: Tatsuyuki Ishi Date: Tue, 6 Mar 2018 13:33:36 +0900 Subject: [PATCH 02/10] rustc_driver: get rid of extra thread on Windows --- src/librustc_driver/lib.rs | 6 +++++- src/rustc/rustc.rs | 9 +++++++++ 2 files changed, 14 insertions(+), 1 deletion(-) diff --git a/src/librustc_driver/lib.rs b/src/librustc_driver/lib.rs index 8605764497a16..761797239412e 100644 --- a/src/librustc_driver/lib.rs +++ b/src/librustc_driver/lib.rs @@ -1488,7 +1488,11 @@ pub fn in_rustc_thread(f: F) -> Result> } }; - #[cfg(not(unix))] + // We set the stack size at link time. See src/rustc/rustc.rs. + #[cfg(windows)] + let spawn_thread = false; + + #[cfg(not(any(windows,unix)))] let spawn_thread = true; // The or condition is added from backward compatibility. diff --git a/src/rustc/rustc.rs b/src/rustc/rustc.rs index bfd01146d2c46..bd36aaf01f860 100644 --- a/src/rustc/rustc.rs +++ b/src/rustc/rustc.rs @@ -9,6 +9,15 @@ // except according to those terms. #![feature(rustc_private)] +#![feature(link_args)] + +// Set the stack size at link time on Windows. See rustc_driver::in_rustc_thread +// for the rationale. +#[cfg_attr(all(windows, target_env = "msvc"), link_args = "/STACK:16777216")] +// We only build for msvc and gnu now, but we use a exhaustive condition here +// so we can expect either the stack size to be set or the build fails. +#[cfg_attr(all(windows, not(target_env = "msvc")), link_args = "-Wl,--stack,16777216")] +extern {} extern crate rustc_driver; From a185b56b7caca17c7aa9d6f702fe1b2209c82e4e Mon Sep 17 00:00:00 2001 From: Tatsuyuki Ishi Date: Sun, 18 Mar 2018 23:02:06 +0900 Subject: [PATCH 03/10] Address review comments --- src/librustc_driver/lib.rs | 8 +++++--- src/libstd/rt.rs | 15 +++++++++++++++ src/libstd/sys/unix/thread.rs | 20 +++++++++++++++++++- src/libstd/thread/mod.rs | 8 -------- 4 files changed, 39 insertions(+), 12 deletions(-) diff --git a/src/librustc_driver/lib.rs b/src/librustc_driver/lib.rs index 761797239412e..e39a2c2f5dcd9 100644 --- a/src/librustc_driver/lib.rs +++ b/src/librustc_driver/lib.rs @@ -1476,13 +1476,15 @@ pub fn in_rustc_thread(f: F) -> Result> } else if rlim.rlim_max < STACK_SIZE as libc::rlim_t { true } else { + std::rt::deinit_stack_guard(); rlim.rlim_cur = STACK_SIZE as libc::rlim_t; if libc::setrlimit(libc::RLIMIT_STACK, &mut rlim) != 0 { let err = io::Error::last_os_error(); - error!("in_rustc_thread: error calling setrlimit: {}", err); - true + // We have already deinited the stack. Further corruption is + // not allowed. + panic!("in_rustc_thread: error calling setrlimit: {}", err); } else { - std::thread::update_stack_guard(); + std::rt::update_stack_guard(); false } } diff --git a/src/libstd/rt.rs b/src/libstd/rt.rs index e1392762a59dc..8f945470b7e94 100644 --- a/src/libstd/rt.rs +++ b/src/libstd/rt.rs @@ -73,3 +73,18 @@ fn lang_start { lang_start_internal(&move || main().report(), argc, argv) } + +/// Function used for reverting changes to the main stack before setrlimit(). +/// This is POSIX (non-Linux) specific and unlikely to be directly stabilized. +#[unstable(feature = "rustc_stack_internals", issue = "0")] +pub unsafe fn deinit_stack_guard() { + ::sys::thread::guard::deinit(); +} + +/// Function used for resetting the main stack guard address after setrlimit(). +/// This is POSIX specific and unlikely to be directly stabilized. +#[unstable(feature = "rustc_stack_internals", issue = "0")] +pub unsafe fn update_stack_guard() { + let main_guard = ::sys::thread::guard::init(); + ::sys_common::thread_info::reset_guard(main_guard); +} diff --git a/src/libstd/sys/unix/thread.rs b/src/libstd/sys/unix/thread.rs index 72cdb9440b8e7..d94e11a5207cb 100644 --- a/src/libstd/sys/unix/thread.rs +++ b/src/libstd/sys/unix/thread.rs @@ -222,7 +222,7 @@ pub mod guard { #[cfg_attr(test, allow(dead_code))] pub mod guard { use libc; - use libc::mmap; + use libc::{mmap, munmap}; use libc::{PROT_NONE, MAP_PRIVATE, MAP_ANON, MAP_FAILED, MAP_FIXED}; use ops::Range; use sys::os; @@ -336,6 +336,24 @@ pub mod guard { } } + pub unsafe fn deinit() { + if !cfg!(target_os = "linux") { + if let Some(mut stackaddr) = get_stack_start() { + // Ensure address is aligned. Same as above. + let remainder = (stackaddr as usize) % PAGE_SIZE; + if remainder != 0 { + stackaddr = ((stackaddr as usize) + PAGE_SIZE - remainder) + as *mut libc::c_void; + } + + // Undo the guard page mapping. + if munmap(stackaddr, PAGE_SIZE) != 0 { + panic!("unable to deallocate the guard page"); + } + } + } + } + #[cfg(any(target_os = "macos", target_os = "bitrig", target_os = "openbsd", diff --git a/src/libstd/thread/mod.rs b/src/libstd/thread/mod.rs index b686ddc205ea7..71aee673cfe3e 100644 --- a/src/libstd/thread/mod.rs +++ b/src/libstd/thread/mod.rs @@ -208,14 +208,6 @@ pub use self::local::{LocalKey, AccessError}; #[unstable(feature = "libstd_thread_internals", issue = "0")] #[doc(hidden)] pub use self::local::os::Key as __OsLocalKeyInner; -/// Function used for resetting the main stack guard address after setrlimit(). -/// This is POSIX specific and unlikely to be directly stabilized. -#[unstable(feature = "rustc_stack_internals", issue = "0")] -pub unsafe fn update_stack_guard() { - let main_guard = imp::guard::init(); - thread_info::reset_guard(main_guard); -} - //////////////////////////////////////////////////////////////////////////////// // Builder //////////////////////////////////////////////////////////////////////////////// From 8c8a72f8224b9cbf283cd120d87397c032ed62f1 Mon Sep 17 00:00:00 2001 From: Tatsuyuki Ishi Date: Tue, 20 Mar 2018 21:15:21 +0900 Subject: [PATCH 04/10] Reinit the stack guard on unexpected failure --- src/librustc_driver/lib.rs | 6 +++--- 1 file changed, 3 insertions(+), 3 deletions(-) diff --git a/src/librustc_driver/lib.rs b/src/librustc_driver/lib.rs index e39a2c2f5dcd9..e56c9928dccd6 100644 --- a/src/librustc_driver/lib.rs +++ b/src/librustc_driver/lib.rs @@ -1480,9 +1480,9 @@ pub fn in_rustc_thread(f: F) -> Result> rlim.rlim_cur = STACK_SIZE as libc::rlim_t; if libc::setrlimit(libc::RLIMIT_STACK, &mut rlim) != 0 { let err = io::Error::last_os_error(); - // We have already deinited the stack. Further corruption is - // not allowed. - panic!("in_rustc_thread: error calling setrlimit: {}", err); + error!("in_rustc_thread: error calling setrlimit: {}", err); + std::rt::update_stack_guard(); + true } else { std::rt::update_stack_guard(); false From 2928c7a8a253b655132ac9f2beb4ca74540f0e14 Mon Sep 17 00:00:00 2001 From: Tatsuyuki Ishi Date: Tue, 20 Mar 2018 21:22:45 +0900 Subject: [PATCH 05/10] Refactor the stack addr aligning code into a function --- src/libstd/sys/unix/thread.rs | 32 ++++++++++++++++---------------- 1 file changed, 16 insertions(+), 16 deletions(-) diff --git a/src/libstd/sys/unix/thread.rs b/src/libstd/sys/unix/thread.rs index d94e11a5207cb..4364089a18184 100644 --- a/src/libstd/sys/unix/thread.rs +++ b/src/libstd/sys/unix/thread.rs @@ -284,10 +284,10 @@ pub mod guard { ret } - pub unsafe fn init() -> Option { - PAGE_SIZE = os::page_size(); - - let mut stackaddr = get_stack_start()?; + // Precondition: PAGE_SIZE is initialized. + unsafe fn get_stack_start_aligned() -> Option<*mut libc::c_void> { + assert!(PAGE_SIZE != 0); + let stackaddr = get_stack_start()?; // Ensure stackaddr is page aligned! A parent process might // have reset RLIMIT_STACK to be non-page aligned. The @@ -296,10 +296,17 @@ pub mod guard { // page-aligned, calculate the fix such that stackaddr < // new_page_aligned_stackaddr < stackaddr + stacksize let remainder = (stackaddr as usize) % PAGE_SIZE; - if remainder != 0 { - stackaddr = ((stackaddr as usize) + PAGE_SIZE - remainder) - as *mut libc::c_void; - } + Some(if remainder == 0 { + stackaddr + } else { + ((stackaddr as usize) + PAGE_SIZE - remainder) as *mut libc::c_void + }) + } + + pub unsafe fn init() -> Option { + PAGE_SIZE = os::page_size(); + + let stackaddr = get_stack_start_aligned()?; if cfg!(target_os = "linux") { // Linux doesn't allocate the whole stack right away, and @@ -338,14 +345,7 @@ pub mod guard { pub unsafe fn deinit() { if !cfg!(target_os = "linux") { - if let Some(mut stackaddr) = get_stack_start() { - // Ensure address is aligned. Same as above. - let remainder = (stackaddr as usize) % PAGE_SIZE; - if remainder != 0 { - stackaddr = ((stackaddr as usize) + PAGE_SIZE - remainder) - as *mut libc::c_void; - } - + if let Some(stackaddr) = get_stack_start_aligned() { // Undo the guard page mapping. if munmap(stackaddr, PAGE_SIZE) != 0 { panic!("unable to deallocate the guard page"); From 91279904345e2ecd3f52b17339183bbe3bd11203 Mon Sep 17 00:00:00 2001 From: Tatsuyuki Ishi Date: Sat, 24 Mar 2018 13:47:36 +0900 Subject: [PATCH 06/10] Fix build on non-Unix platforms --- src/libstd/sys/cloudabi/thread.rs | 1 + src/libstd/sys/redox/thread.rs | 1 + src/libstd/sys/unix/thread.rs | 1 + src/libstd/sys/wasm/thread.rs | 1 + src/libstd/sys/windows/thread.rs | 1 + 5 files changed, 5 insertions(+) diff --git a/src/libstd/sys/cloudabi/thread.rs b/src/libstd/sys/cloudabi/thread.rs index 78a3b82546e3a..a22d9053b6964 100644 --- a/src/libstd/sys/cloudabi/thread.rs +++ b/src/libstd/sys/cloudabi/thread.rs @@ -118,6 +118,7 @@ pub mod guard { pub unsafe fn init() -> Option { None } + pub unsafe fn deinit() {} } fn min_stack_size(_: *const libc::pthread_attr_t) -> usize { diff --git a/src/libstd/sys/redox/thread.rs b/src/libstd/sys/redox/thread.rs index c4719a94c7e9d..f20350269b7cf 100644 --- a/src/libstd/sys/redox/thread.rs +++ b/src/libstd/sys/redox/thread.rs @@ -91,4 +91,5 @@ pub mod guard { pub type Guard = !; pub unsafe fn current() -> Option { None } pub unsafe fn init() -> Option { None } + pub unsafe fn deinit() {} } diff --git a/src/libstd/sys/unix/thread.rs b/src/libstd/sys/unix/thread.rs index 4364089a18184..b9ec0d4a40338 100644 --- a/src/libstd/sys/unix/thread.rs +++ b/src/libstd/sys/unix/thread.rs @@ -209,6 +209,7 @@ pub mod guard { pub type Guard = Range; pub unsafe fn current() -> Option { None } pub unsafe fn init() -> Option { None } + pub unsafe fn deinit() {} } diff --git a/src/libstd/sys/wasm/thread.rs b/src/libstd/sys/wasm/thread.rs index 6a066509b492a..7345843b975e4 100644 --- a/src/libstd/sys/wasm/thread.rs +++ b/src/libstd/sys/wasm/thread.rs @@ -46,4 +46,5 @@ pub mod guard { pub type Guard = !; pub unsafe fn current() -> Option { None } pub unsafe fn init() -> Option { None } + pub unsafe fn deinit() {} } diff --git a/src/libstd/sys/windows/thread.rs b/src/libstd/sys/windows/thread.rs index 43abfbb1f645e..4b3d1b586b570 100644 --- a/src/libstd/sys/windows/thread.rs +++ b/src/libstd/sys/windows/thread.rs @@ -96,4 +96,5 @@ pub mod guard { pub type Guard = !; pub unsafe fn current() -> Option { None } pub unsafe fn init() -> Option { None } + pub unsafe fn deinit() {} } From d39b02c2c9e8aa472ab47ccfb5de63bc616daf62 Mon Sep 17 00:00:00 2001 From: Tatsuyuki Ishi Date: Sun, 25 Mar 2018 16:08:03 +0900 Subject: [PATCH 07/10] Use a more conservative way to deinit stack guard --- src/libstd/sys/unix/thread.rs | 20 +++++++++++++++----- 1 file changed, 15 insertions(+), 5 deletions(-) diff --git a/src/libstd/sys/unix/thread.rs b/src/libstd/sys/unix/thread.rs index b9ec0d4a40338..6064b55489efb 100644 --- a/src/libstd/sys/unix/thread.rs +++ b/src/libstd/sys/unix/thread.rs @@ -223,8 +223,8 @@ pub mod guard { #[cfg_attr(test, allow(dead_code))] pub mod guard { use libc; - use libc::{mmap, munmap}; - use libc::{PROT_NONE, MAP_PRIVATE, MAP_ANON, MAP_FAILED, MAP_FIXED}; + use libc::mmap; + use libc::{PROT_NONE, PROT_READ, PROT_WRITE, MAP_PRIVATE, MAP_ANON, MAP_FAILED, MAP_FIXED}; use ops::Range; use sys::os; @@ -347,9 +347,19 @@ pub mod guard { pub unsafe fn deinit() { if !cfg!(target_os = "linux") { if let Some(stackaddr) = get_stack_start_aligned() { - // Undo the guard page mapping. - if munmap(stackaddr, PAGE_SIZE) != 0 { - panic!("unable to deallocate the guard page"); + // Remove the protection on the guard page. + // FIXME: we cannot unmap the page, because when we mmap() + // above it may be already mapped by the OS, which we can't + // detect from mmap()'s return value. If we unmap this page, + // it will lead to failure growing stack size on platforms like + // macOS. Instead, just restore the page to a writable state. + // This ain't Linux, so we probably don't need to care about + // execstack. + let result = mmap(stackaddr, PAGE_SIZE, PROT_READ | PROT_WRITE, + MAP_PRIVATE | MAP_ANON | MAP_FIXED, -1, 0); + + if result != stackaddr || result == MAP_FAILED { + panic!("unable to reset the guard page"); } } } From 8ecbec1dba048bf55461dbff772f18a72311ecc2 Mon Sep 17 00:00:00 2001 From: Tatsuyuki Ishi Date: Wed, 28 Mar 2018 18:47:16 +0900 Subject: [PATCH 08/10] Use mprotect instead of mmap --- src/libstd/sys/unix/thread.rs | 5 ++--- 1 file changed, 2 insertions(+), 3 deletions(-) diff --git a/src/libstd/sys/unix/thread.rs b/src/libstd/sys/unix/thread.rs index 6064b55489efb..775289f393b04 100644 --- a/src/libstd/sys/unix/thread.rs +++ b/src/libstd/sys/unix/thread.rs @@ -355,10 +355,9 @@ pub mod guard { // macOS. Instead, just restore the page to a writable state. // This ain't Linux, so we probably don't need to care about // execstack. - let result = mmap(stackaddr, PAGE_SIZE, PROT_READ | PROT_WRITE, - MAP_PRIVATE | MAP_ANON | MAP_FIXED, -1, 0); + let result = mprotect(stackaddr, PAGE_SIZE, PROT_READ | PROT_WRITE); - if result != stackaddr || result == MAP_FAILED { + if result != 0 { panic!("unable to reset the guard page"); } } From 50ca86be2438369dd1263c619a3bac21bd30a37f Mon Sep 17 00:00:00 2001 From: Tatsuyuki Ishi Date: Wed, 28 Mar 2018 18:51:25 +0900 Subject: [PATCH 09/10] Set link args for rustdoc --- src/rustc/rustc.rs | 1 + src/tools/rustdoc/main.rs | 10 ++++++++++ 2 files changed, 11 insertions(+) diff --git a/src/rustc/rustc.rs b/src/rustc/rustc.rs index bd36aaf01f860..9fa33f911a168 100644 --- a/src/rustc/rustc.rs +++ b/src/rustc/rustc.rs @@ -17,6 +17,7 @@ // We only build for msvc and gnu now, but we use a exhaustive condition here // so we can expect either the stack size to be set or the build fails. #[cfg_attr(all(windows, not(target_env = "msvc")), link_args = "-Wl,--stack,16777216")] +// Also, don't forget to set this for rustdoc. extern {} extern crate rustc_driver; diff --git a/src/tools/rustdoc/main.rs b/src/tools/rustdoc/main.rs index 9c37e249ba8cc..e726dea84f103 100644 --- a/src/tools/rustdoc/main.rs +++ b/src/tools/rustdoc/main.rs @@ -8,6 +8,16 @@ // option. This file may not be copied, modified, or distributed // except according to those terms. +#![feature(link_args)] +// Set the stack size at link time on Windows. See rustc_driver::in_rustc_thread +// for the rationale. +#[cfg_attr(all(windows, target_env = "msvc"), link_args = "/STACK:16777216")] +// We only build for msvc and gnu now, but we use a exhaustive condition here +// so we can expect either the stack size to be set or the build fails. +#[cfg_attr(all(windows, not(target_env = "msvc")), link_args = "-Wl,--stack,16777216")] +// See src/rustc/rustc.rs for the corresponding rustc settings. +extern {} + extern crate rustdoc; fn main() { rustdoc::main() } From 7db854b36f9598e44fb4d61428d42d7769233e19 Mon Sep 17 00:00:00 2001 From: Tatsuyuki Ishi Date: Tue, 3 Apr 2018 12:42:36 +0900 Subject: [PATCH 10/10] Fix imports --- src/libstd/sys/unix/thread.rs | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/src/libstd/sys/unix/thread.rs b/src/libstd/sys/unix/thread.rs index 775289f393b04..2db3d4a5744e7 100644 --- a/src/libstd/sys/unix/thread.rs +++ b/src/libstd/sys/unix/thread.rs @@ -223,7 +223,7 @@ pub mod guard { #[cfg_attr(test, allow(dead_code))] pub mod guard { use libc; - use libc::mmap; + use libc::{mmap, mprotect}; use libc::{PROT_NONE, PROT_READ, PROT_WRITE, MAP_PRIVATE, MAP_ANON, MAP_FAILED, MAP_FIXED}; use ops::Range; use sys::os;