about summary refs log tree commit diff
path: root/src/librustc_data_structures
diff options
context:
space:
mode:
authorbors <bors@rust-lang.org>2020-05-07 00:03:23 +0000
committerbors <bors@rust-lang.org>2020-05-07 00:03:23 +0000
commit97f3eeec8216d7155c24674b9be55e7c672bcae3 (patch)
tree5e5b22df21838a464ae5cf900d0b4a27855edc3c /src/librustc_data_structures
parent29457dd92c5c754027c18b28c9e1307a345aafec (diff)
parent935a05f1beaa5ed872e66e521e510bba61509b48 (diff)
downloadrust-97f3eeec8216d7155c24674b9be55e7c672bcae3.tar.gz
rust-97f3eeec8216d7155c24674b9be55e7c672bcae3.zip
Auto merge of #55617 - oli-obk:stacker, r=nagisa,oli-obk
Prevent compiler stack overflow for deeply recursive code

I was unable to write a test that

1. runs in under 1s
2. overflows on my machine without this patch

The following reproduces the issue, but I don't think it's sensible to include a test that takes 30s to compile. We can now easily squash newly appearing overflows by the strategic insertion of calls to `ensure_sufficient_stack`.

```rust
// compile-pass

#![recursion_limit="1000000"]

macro_rules! chain {
    (EE $e:expr) => {$e.sin()};
    (RECURSE $i:ident $e:expr) => {chain!($i chain!($i chain!($i chain!($i $e))))};
    (Z $e:expr) => {chain!(RECURSE EE $e)};
    (Y $e:expr) => {chain!(RECURSE Z $e)};
    (X $e:expr) => {chain!(RECURSE Y $e)};
    (A $e:expr) => {chain!(RECURSE X $e)};
    (B $e:expr) => {chain!(RECURSE A $e)};
    (C $e:expr) => {chain!(RECURSE B $e)};
    // causes overflow on x86_64 linux
    // less than 1 second until overflow on test machine
    // after overflow has been fixed, takes 30s to compile :/
    (D $e:expr) => {chain!(RECURSE C $e)};
    (E $e:expr) => {chain!(RECURSE D $e)};
    (F $e:expr) => {chain!(RECURSE E $e)};
    // more than 10 seconds
    (G $e:expr) => {chain!(RECURSE F $e)};
    (H $e:expr) => {chain!(RECURSE G $e)};
    (I $e:expr) => {chain!(RECURSE H $e)};
    (J $e:expr) => {chain!(RECURSE I $e)};
    (K $e:expr) => {chain!(RECURSE J $e)};
    (L $e:expr) => {chain!(RECURSE L $e)};
}

fn main() {
    let x = chain!(D 42.0_f32);
}
```

fixes #55471
fixes #41884
fixes #40161
fixes #34844
fixes #32594

cc @alexcrichton @rust-lang/compiler

I looked at all code that checks the recursion limit and inserted stack growth calls where appropriate.
Diffstat (limited to 'src/librustc_data_structures')
-rw-r--r--src/librustc_data_structures/Cargo.toml1
-rw-r--r--src/librustc_data_structures/lib.rs1
-rw-r--r--src/librustc_data_structures/stack.rs17
3 files changed, 19 insertions, 0 deletions
diff --git a/src/librustc_data_structures/Cargo.toml b/src/librustc_data_structures/Cargo.toml
index e257ada0629..f543f8051a4 100644
--- a/src/librustc_data_structures/Cargo.toml
+++ b/src/librustc_data_structures/Cargo.toml
@@ -28,6 +28,7 @@ rustc_index = { path = "../librustc_index", package = "rustc_index" }
 bitflags = "1.2.1"
 measureme = "0.7.1"
 libc = "0.2"
+stacker = "0.1.6"
 
 [dependencies.parking_lot]
 version = "0.10"
diff --git a/src/librustc_data_structures/lib.rs b/src/librustc_data_structures/lib.rs
index a7bee8a067c..9164734783c 100644
--- a/src/librustc_data_structures/lib.rs
+++ b/src/librustc_data_structures/lib.rs
@@ -80,6 +80,7 @@ pub mod stable_set;
 #[macro_use]
 pub mod stable_hasher;
 pub mod sharded;
+pub mod stack;
 pub mod sync;
 pub mod thin_vec;
 pub mod tiny_list;
diff --git a/src/librustc_data_structures/stack.rs b/src/librustc_data_structures/stack.rs
new file mode 100644
index 00000000000..a4964b7aa0c
--- /dev/null
+++ b/src/librustc_data_structures/stack.rs
@@ -0,0 +1,17 @@
+// This is the amount of bytes that need to be left on the stack before increasing the size.
+// It must be at least as large as the stack required by any code that does not call
+// `ensure_sufficient_stack`.
+const RED_ZONE: usize = 100 * 1024; // 100k
+
+// Only the first stack that is pushed, grows exponentially (2^n * STACK_PER_RECURSION) from then
+// on. This flag has performance relevant characteristics. Don't set it too high.
+const STACK_PER_RECURSION: usize = 1 * 1024 * 1024; // 1MB
+
+/// Grows the stack on demand to prevent stack overflow. Call this in strategic locations
+/// to "break up" recursive calls. E.g. almost any call to `visit_expr` or equivalent can benefit
+/// from this.
+///
+/// Should not be sprinkled around carelessly, as it causes a little bit of overhead.
+pub fn ensure_sufficient_stack<R>(f: impl FnOnce() -> R) -> R {
+    stacker::maybe_grow(RED_ZONE, STACK_PER_RECURSION, f)
+}