Skip to content
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

rustc_driver: get rid of the extra thread #48575

Merged
merged 10 commits into from
Apr 4, 2018
57 changes: 49 additions & 8 deletions src/librustc_driver/lib.rs
Original file line number Diff line number Diff line change
Expand Up @@ -24,6 +24,7 @@
#![feature(quote)]
#![feature(rustc_diagnostic_macros)]
#![feature(set_stdio)]
#![feature(rustc_stack_internals)]

extern crate arena;
extern crate getopts;
Expand Down Expand Up @@ -1461,16 +1462,56 @@ pub fn in_rustc_thread<F, R>(f: F) -> Result<R, Box<Any + Send>>
// 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 {
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);
std::rt::update_stack_guard();
true
} else {
std::rt::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);
}
// 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;

let thread = cfg.spawn(f);
thread.unwrap().join()
// 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() {
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Could this be checked earlier? If this env var is set I think we'll want to force spawning a thread.

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I disagree: that environment variable only affects stack size of non-main thread. If we can extend the main thread stack, that's all what we need.

Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Without moving this check earlier it's breaking existing behavior. Today the stack size of the rustc thread can be controlled by configuring RUST_MIN_STACK (can be useful for debugging). This commit breaks that ability because setrlimit will probably succeed on Linux.

cfg = cfg.stack_size(STACK_SIZE);
}

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.
Expand Down
15 changes: 15 additions & 0 deletions src/libstd/rt.rs
Original file line number Diff line number Diff line change
Expand Up @@ -73,3 +73,18 @@ fn lang_start<T: ::process::Termination + 'static>
{
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);
}
1 change: 1 addition & 0 deletions src/libstd/sys/cloudabi/thread.rs
Original file line number Diff line number Diff line change
Expand Up @@ -118,6 +118,7 @@ pub mod guard {
pub unsafe fn init() -> Option<Guard> {
None
}
pub unsafe fn deinit() {}
}

fn min_stack_size(_: *const libc::pthread_attr_t) -> usize {
Expand Down
1 change: 1 addition & 0 deletions src/libstd/sys/redox/thread.rs
Original file line number Diff line number Diff line change
Expand Up @@ -91,4 +91,5 @@ pub mod guard {
pub type Guard = !;
pub unsafe fn current() -> Option<Guard> { None }
pub unsafe fn init() -> Option<Guard> { None }
pub unsafe fn deinit() {}
}
47 changes: 38 additions & 9 deletions src/libstd/sys/unix/thread.rs
Original file line number Diff line number Diff line change
Expand Up @@ -209,6 +209,7 @@ pub mod guard {
pub type Guard = Range<usize>;
pub unsafe fn current() -> Option<Guard> { None }
pub unsafe fn init() -> Option<Guard> { None }
pub unsafe fn deinit() {}
}


Expand All @@ -223,7 +224,7 @@ pub mod guard {
pub mod guard {
use libc;
use libc::mmap;
use libc::{PROT_NONE, MAP_PRIVATE, MAP_ANON, MAP_FAILED, MAP_FIXED};
use libc::{PROT_NONE, PROT_READ, PROT_WRITE, MAP_PRIVATE, MAP_ANON, MAP_FAILED, MAP_FIXED};
use ops::Range;
use sys::os;

Expand Down Expand Up @@ -284,10 +285,10 @@ pub mod guard {
ret
}

pub unsafe fn init() -> Option<Guard> {
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
Expand All @@ -296,10 +297,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<Guard> {
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
Expand Down Expand Up @@ -336,6 +344,27 @@ pub mod guard {
}
}

pub unsafe fn deinit() {
if !cfg!(target_os = "linux") {
if let Some(stackaddr) = get_stack_start_aligned() {
// 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,
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Wouldn't mprotect be more appropriate here?

MAP_PRIVATE | MAP_ANON | MAP_FIXED, -1, 0);

if result != stackaddr || result == MAP_FAILED {
panic!("unable to reset the guard page");
}
}
}
}

#[cfg(any(target_os = "macos",
target_os = "bitrig",
target_os = "openbsd",
Expand Down
1 change: 1 addition & 0 deletions src/libstd/sys/wasm/thread.rs
Original file line number Diff line number Diff line change
Expand Up @@ -46,4 +46,5 @@ pub mod guard {
pub type Guard = !;
pub unsafe fn current() -> Option<Guard> { None }
pub unsafe fn init() -> Option<Guard> { None }
pub unsafe fn deinit() {}
}
1 change: 1 addition & 0 deletions src/libstd/sys/windows/thread.rs
Original file line number Diff line number Diff line change
Expand Up @@ -96,4 +96,5 @@ pub mod guard {
pub type Guard = !;
pub unsafe fn current() -> Option<Guard> { None }
pub unsafe fn init() -> Option<Guard> { None }
pub unsafe fn deinit() {}
}
4 changes: 4 additions & 0 deletions src/libstd/sys_common/thread_info.rs
Original file line number Diff line number Diff line change
Expand Up @@ -50,3 +50,7 @@ pub fn set(stack_guard: Option<Guard>, thread: Thread) {
thread,
}));
}

pub fn reset_guard(stack_guard: Option<Guard>) {
THREAD_INFO.with(move |c| c.borrow_mut().as_mut().unwrap().stack_guard = stack_guard);
}
9 changes: 9 additions & 0 deletions src/rustc/rustc.rs
Original file line number Diff line number Diff line change
Expand Up @@ -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;

Expand Down