Skip to content

Commit

Permalink
Rollup merge of rust-lang#125302 - workingjubilee:prefer-my-stack-nea…
Browse files Browse the repository at this point in the history
…t, r=compiler-errors

defrost `RUST_MIN_STACK=ice rustc hello.rs`

I didn't think too hard about testing my previous PR rust-lang#122847 which makes our stack overflow handler assist people in discovering the `RUST_MIN_STACK` variable (which apparently is surprisingly useful for Really Big codebases). After it was merged, some useful comments left in a drive-by review led me to discover I had added an ICE. This reworks the code a bit to explain the rationale, remove the ICE that I introduced, and properly test one of the diagnostics.
  • Loading branch information
matthiaskrgr authored May 20, 2024
2 parents 199d3bf + b6d0d6d commit ecbd110
Show file tree
Hide file tree
Showing 7 changed files with 46 additions and 10 deletions.
1 change: 1 addition & 0 deletions compiler/rustc_interface/src/interface.rs
Original file line number Diff line number Diff line change
Expand Up @@ -389,6 +389,7 @@ pub fn run_compiler<R: Send>(config: Config, f: impl FnOnce(&Compiler) -> R + Se
let hash_kind = config.opts.unstable_opts.src_hash_algorithm(&target);

util::run_in_thread_pool_with_globals(
&early_dcx,
config.opts.edition,
config.opts.unstable_opts.threads,
SourceMapInputs { file_loader, path_mapping, hash_kind },
Expand Down
41 changes: 32 additions & 9 deletions compiler/rustc_interface/src/util.rs
Original file line number Diff line number Diff line change
Expand Up @@ -51,20 +51,38 @@ pub fn add_configuration(cfg: &mut Cfg, sess: &mut Session, codegen_backend: &dy
pub static STACK_SIZE: OnceLock<usize> = OnceLock::new();
pub const DEFAULT_STACK_SIZE: usize = 8 * 1024 * 1024;

fn init_stack_size() -> usize {
fn init_stack_size(early_dcx: &EarlyDiagCtxt) -> usize {
// Obey the environment setting or default
*STACK_SIZE.get_or_init(|| {
env::var_os("RUST_MIN_STACK")
.map(|os_str| os_str.to_string_lossy().into_owned())
// ignore if it is set to nothing
.filter(|s| s.trim() != "")
.map(|s| s.trim().parse::<usize>().unwrap())
.as_ref()
.map(|os_str| os_str.to_string_lossy())
// if someone finds out `export RUST_MIN_STACK=640000` isn't enough stack
// they might try to "unset" it by running `RUST_MIN_STACK= rustc code.rs`
// this is wrong, but std would nonetheless "do what they mean", so let's do likewise
.filter(|s| !s.trim().is_empty())
// rustc is a batch program, so error early on inputs which are unlikely to be intended
// so no one thinks we parsed them setting `RUST_MIN_STACK="64 megabytes"`
// FIXME: we could accept `RUST_MIN_STACK=64MB`, perhaps?
.map(|s| {
let s = s.trim();
// FIXME(workingjubilee): add proper diagnostics when we factor out "pre-run" setup
#[allow(rustc::untranslatable_diagnostic, rustc::diagnostic_outside_of_impl)]
s.parse::<usize>().unwrap_or_else(|_| {
let mut err = early_dcx.early_struct_fatal(format!(
r#"`RUST_MIN_STACK` should be a number of bytes, but was "{s}""#,
));
err.note("you can also unset `RUST_MIN_STACK` to use the default stack size");
err.emit()
})
})
// otherwise pick a consistent default
.unwrap_or(DEFAULT_STACK_SIZE)
})
}

fn run_in_thread_with_globals<F: FnOnce(CurrentGcx) -> R + Send, R: Send>(
thread_stack_size: usize,
edition: Edition,
sm_inputs: SourceMapInputs,
f: F,
Expand All @@ -75,7 +93,7 @@ fn run_in_thread_with_globals<F: FnOnce(CurrentGcx) -> R + Send, R: Send>(
// the parallel compiler, in particular to ensure there is no accidental
// sharing of data between the main thread and the compilation thread
// (which might cause problems for the parallel compiler).
let builder = thread::Builder::new().name("rustc".to_string()).stack_size(init_stack_size());
let builder = thread::Builder::new().name("rustc".to_string()).stack_size(thread_stack_size);

// We build the session globals and run `f` on the spawned thread, because
// `SessionGlobals` does not impl `Send` in the non-parallel compiler.
Expand All @@ -100,16 +118,19 @@ fn run_in_thread_with_globals<F: FnOnce(CurrentGcx) -> R + Send, R: Send>(

#[cfg(not(parallel_compiler))]
pub(crate) fn run_in_thread_pool_with_globals<F: FnOnce(CurrentGcx) -> R + Send, R: Send>(
thread_builder_diag: &EarlyDiagCtxt,
edition: Edition,
_threads: usize,
sm_inputs: SourceMapInputs,
f: F,
) -> R {
run_in_thread_with_globals(edition, sm_inputs, f)
let thread_stack_size = init_stack_size(thread_builder_diag);
run_in_thread_with_globals(thread_stack_size, edition, sm_inputs, f)
}

#[cfg(parallel_compiler)]
pub(crate) fn run_in_thread_pool_with_globals<F: FnOnce(CurrentGcx) -> R + Send, R: Send>(
thread_builder_diag: &EarlyDiagCtxt,
edition: Edition,
threads: usize,
sm_inputs: SourceMapInputs,
Expand All @@ -121,10 +142,12 @@ pub(crate) fn run_in_thread_pool_with_globals<F: FnOnce(CurrentGcx) -> R + Send,
use rustc_query_system::query::{break_query_cycles, QueryContext};
use std::process;

let thread_stack_size = init_stack_size(thread_builder_diag);

let registry = sync::Registry::new(std::num::NonZero::new(threads).unwrap());

if !sync::is_dyn_thread_safe() {
return run_in_thread_with_globals(edition, sm_inputs, |current_gcx| {
return run_in_thread_with_globals(thread_stack_size, edition, sm_inputs, |current_gcx| {
// Register the thread for use with the `WorkerLocal` type.
registry.register();

Expand Down Expand Up @@ -167,7 +190,7 @@ pub(crate) fn run_in_thread_pool_with_globals<F: FnOnce(CurrentGcx) -> R + Send,
})
.unwrap();
})
.stack_size(init_stack_size());
.stack_size(thread_stack_size);

// We create the session globals on the main thread, then create the thread
// pool. Upon creation, each worker thread created gets a copy of the
Expand Down
6 changes: 6 additions & 0 deletions tests/ui/rustc-env/README.md
Original file line number Diff line number Diff line change
@@ -0,0 +1,6 @@
Some environment variables affect rustc's behavior not because they are major compiler interfaces
but rather because rustc is, ultimately, a Rust program, with debug logging, stack control, etc.

Prefer to group tests that use environment variables to control something about rustc's core UX,
like "can we parse this number of parens if we raise RUST_MIN_STACK?" with related code for that
compiler feature.
File renamed without changes.
2 changes: 2 additions & 0 deletions tests/ui/rustc-env/min-stack-banana.rs
Original file line number Diff line number Diff line change
@@ -0,0 +1,2 @@
//@ rustc-env:RUST_MIN_STACK=banana
fn main() {}
4 changes: 4 additions & 0 deletions tests/ui/rustc-env/min-stack-banana.stderr
Original file line number Diff line number Diff line change
@@ -0,0 +1,4 @@
error: `RUST_MIN_STACK` should be a number of bytes, but was "banana"
|
= note: you can also unset `RUST_MIN_STACK` to use the default stack size

Original file line number Diff line number Diff line change
Expand Up @@ -5,7 +5,7 @@
//@ dont-check-compiler-stdout
//@ dont-check-compiler-stderr
//@ compile-flags: --error-format human
//@ aux-build: rustc-rust-log-aux.rs
//@ aux-build: rust-log-aux.rs
//@ rustc-env:RUSTC_LOG=debug

fn main() {}

0 comments on commit ecbd110

Please sign in to comment.