Suggest `RUST_MIN_STACK` workaround on overflow #122847

workingjubilee · 2024-03-21T21:49:44Z

For some Rust crates, like p384, we can't do a whole lot about it even if the stack overflow is reported like in #122357 because the problem may be inside LLVM or another codegen backend. We can, however, suggest people set a new RUST_MIN_STACK value while handling the SIGSEGV, as that stack-setting will carry forward into the dylib.

As a bonus, this also leads to cleaning up the stack-setting code a bit.

rustbot · 2024-03-21T21:49:51Z

r? @TaKO8Ki

rustbot has assigned @TaKO8Ki.
They will have a look at your PR within the next two weeks and either review your PR or reassign to another reviewer.

Use r? to explicitly pick a reviewer

TaKO8Ki · 2024-03-22T05:38:17Z

@bors r+ rollup

bors · 2024-03-22T05:38:19Z

📌 Commit 5425338 has been approved by TaKO8Ki

It is now in the queue for this repository.

…ack-workaround-on-overflow, r=TaKO8Ki Suggest `RUST_MIN_STACK` workaround on overflow For some Rust crates, like p384, we can't do a whole lot about it even if the stack overflow is reported like in rust-lang#122357 because the problem may be inside LLVM or another codegen backend. We can, however, suggest people set a new `RUST_MIN_STACK` value while handling the SIGSEGV, as that stack-setting will carry forward into the dylib. As a bonus, this also leads to cleaning up the stack-setting code a bit.

…iaskrgr Rollup of 9 pull requests Successful merges: - rust-lang#121619 (Experimental feature postfix match) - rust-lang#122370 (Gracefully handle `AnonConst` in `diagnostic_hir_wf_check()`) - rust-lang#122537 (interpret/allocation: fix aliasing issue in interpreter and refactor getters a bit) - rust-lang#122542 (coverage: Clean up marker statements that aren't needed later) - rust-lang#122800 (Add `NonNull::<[T]>::is_empty`.) - rust-lang#122820 (Stop using `<DefId as Ord>` in various diagnostic situations) - rust-lang#122847 (Suggest `RUST_MIN_STACK` workaround on overflow) - rust-lang#122855 (Fix Itanium mangling usizes) - rust-lang#122863 (add more ice tests ) r? `@ghost` `@rustbot` modify labels: rollup

Rollup merge of rust-lang#122847 - workingjubilee:suggest-rust-min-stack-workaround-on-overflow, r=TaKO8Ki Suggest `RUST_MIN_STACK` workaround on overflow For some Rust crates, like p384, we can't do a whole lot about it even if the stack overflow is reported like in rust-lang#122357 because the problem may be inside LLVM or another codegen backend. We can, however, suggest people set a new `RUST_MIN_STACK` value while handling the SIGSEGV, as that stack-setting will carry forward into the dylib. As a bonus, this also leads to cleaning up the stack-setting code a bit.

…iaskrgr Rollup of 9 pull requests Successful merges: - rust-lang#121619 (Experimental feature postfix match) - rust-lang#122370 (Gracefully handle `AnonConst` in `diagnostic_hir_wf_check()`) - rust-lang#122537 (interpret/allocation: fix aliasing issue in interpreter and refactor getters a bit) - rust-lang#122542 (coverage: Clean up marker statements that aren't needed later) - rust-lang#122800 (Add `NonNull::<[T]>::is_empty`.) - rust-lang#122820 (Stop using `<DefId as Ord>` in various diagnostic situations) - rust-lang#122847 (Suggest `RUST_MIN_STACK` workaround on overflow) - rust-lang#122855 (Fix Itanium mangling usizes) - rust-lang#122863 (add more ice tests ) r? `@ghost` `@rustbot` modify labels: rollup

danielhenrymantilla · 2024-04-12T10:14:04Z

compiler/rustc_interface/src/util.rs

+        env::var_os("RUST_MIN_STACK")
+            .map(|os_str| os_str.to_string_lossy().into_owned())


To avoid the into_owned() cloning:

Suggested change

env::var_os("RUST_MIN_STACK")

.map(|os_str| os_str.to_string_lossy().into_owned())

env::var_os("RUST_MIN_STACK")

.as_ref()

.map(|os_str| os_str.to_string_lossy())

danielhenrymantilla · 2024-04-12T10:16:35Z

compiler/rustc_interface/src/util.rs

+            .filter(|s| s.trim() != "")
+            .map(|s| s.trim().parse::<usize>().unwrap())


Do we want to .unwrap() here? The UX on a bad RUST_MIN_STACK wouldn't be great. Given the usage of .filter() and .unwrap_or() outside of this, does it mean we want to silently ignore bad values (e.g., RUST_MIN_STACK=" ")?

Suggested change

.filter(|s| s.trim() != "")

.map(|s| s.trim().parse::<usize>().unwrap())

.and_then(|s| s.trim().parse::<usize>().ok())

or, if we don't want to silently ignore the erroneous value:

Suggested change

.filter(|s| s.trim() != "")

.map(|s| s.trim().parse::<usize>().unwrap())

.map(|s| s.trim().parse::<usize>().expect("`RUST_MIN_STACK` env var to be a `usize`))

thread 'main' panicked at compiler/rustc_interface/src/util.rs:61:48:
called Result::unwrap() on an Err value: ParseIntError { kind: InvalidDigit }

Huh, I thought for some reason it'd look better than that.

danielhenrymantilla · 2024-04-12T10:27:32Z

Whoops didn't see the PR was already merged 😆

…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.

Rollup merge of rust-lang#125302 - workingjubilee:prefer-my-stack-neat, 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.

workingjubilee added 2 commits March 21, 2024 14:47

Use less hacky STACK_SIZE

60891ca

Suggest using RUST_MIN_STACK if rustc overflowed

5425338

rustbot assigned TaKO8Ki Mar 21, 2024

rustbot added S-waiting-on-review Status: Awaiting review from the assignee but also interested parties. T-compiler Relevant to the compiler team, which will review and decide on the PR/issue. labels Mar 21, 2024

bors added S-waiting-on-bors Status: Waiting on bors to run and complete tests. Bors will change the label on completion. and removed S-waiting-on-review Status: Awaiting review from the assignee but also interested parties. labels Mar 22, 2024

matthiaskrgr mentioned this pull request Mar 22, 2024

Rollup of 8 pull requests #122866

Closed

This was referenced Mar 22, 2024

Rollup of 7 pull requests #122867

Closed

Rollup of 9 pull requests #122869

Merged

bors merged commit b317cda into rust-lang:master Mar 22, 2024
11 checks passed

rustbot added this to the 1.79.0 milestone Mar 22, 2024

workingjubilee deleted the suggest-rust-min-stack-workaround-on-overflow branch March 22, 2024 17:23

This was referenced Mar 26, 2024

Resize gravitational/teleport#39819

Merged

Updates RUST_VERSION and bumps IronRDP rev to latest, adding RUST_MIN_STACK to fix ARM builds gravitational/teleport#39895

Merged

danielhenrymantilla reviewed Apr 12, 2024

View reviewed changes

workingjubilee mentioned this pull request May 20, 2024

defrost RUST_MIN_STACK=ice rustc hello.rs #125302

Merged

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

Suggest `RUST_MIN_STACK` workaround on overflow #122847

Suggest `RUST_MIN_STACK` workaround on overflow #122847

workingjubilee commented Mar 21, 2024

rustbot commented Mar 21, 2024

TaKO8Ki commented Mar 22, 2024

bors commented Mar 22, 2024

danielhenrymantilla Apr 12, 2024

danielhenrymantilla Apr 12, 2024

workingjubilee May 19, 2024

danielhenrymantilla commented Apr 12, 2024

		env::var_os("RUST_MIN_STACK")
		.map(\|os_str\| os_str.to_string_lossy().into_owned())

		.filter(\|s\| s.trim() != "")
		.map(\|s\| s.trim().parse::<usize>().unwrap())

	.filter(\|s\| s.trim() != "")
	.map(\|s\| s.trim().parse::<usize>().unwrap())
	.and_then(\|s\| s.trim().parse::<usize>().ok())

	.filter(\|s\| s.trim() != "")
	.map(\|s\| s.trim().parse::<usize>().unwrap())
	.map(\|s\| s.trim().parse::<usize>().expect("`RUST_MIN_STACK` env var to be a `usize`))

Suggest RUST_MIN_STACK workaround on overflow #122847

Suggest RUST_MIN_STACK workaround on overflow #122847

Conversation

workingjubilee commented Mar 21, 2024

rustbot commented Mar 21, 2024

TaKO8Ki commented Mar 22, 2024

bors commented Mar 22, 2024

danielhenrymantilla Apr 12, 2024

Choose a reason for hiding this comment

danielhenrymantilla Apr 12, 2024

Choose a reason for hiding this comment

workingjubilee May 19, 2024

Choose a reason for hiding this comment

danielhenrymantilla commented Apr 12, 2024

Suggest `RUST_MIN_STACK` workaround on overflow #122847

Suggest `RUST_MIN_STACK` workaround on overflow #122847