Skip to content

Commit

Permalink
Auto merge of #55617 - oli-obk:stacker, r=<try>
Browse files Browse the repository at this point in the history
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.
  • Loading branch information
bors committed Nov 6, 2018
2 parents f90aab7 + 9f61d00 commit 2b10b3d
Show file tree
Hide file tree
Showing 17 changed files with 279 additions and 210 deletions.
18 changes: 18 additions & 0 deletions src/Cargo.lock
Original file line number Diff line number Diff line change
Expand Up @@ -824,6 +824,11 @@ dependencies = [
"termcolor 1.0.2 (registry+https://github.com/rust-lang/crates.io-index)",
]

[[package]]
name = "gcc"
version = "0.3.55"
source = "registry+https://github.com/rust-lang/crates.io-index"

[[package]]
name = "getopts"
version = "0.2.17"
Expand Down Expand Up @@ -1913,6 +1918,7 @@ dependencies = [
"scoped-tls 0.1.2 (registry+https://github.com/rust-lang/crates.io-index)",
"serialize 0.0.0",
"smallvec 0.6.5 (registry+https://github.com/rust-lang/crates.io-index)",
"stacker 0.1.3 (registry+https://github.com/rust-lang/crates.io-index)",
"syntax 0.0.0",
"syntax_pos 0.0.0",
"tempfile 3.0.3 (registry+https://github.com/rust-lang/crates.io-index)",
Expand Down Expand Up @@ -2672,6 +2678,16 @@ name = "stable_deref_trait"
version = "1.1.0"
source = "registry+https://github.com/rust-lang/crates.io-index"

[[package]]
name = "stacker"
version = "0.1.3"
source = "registry+https://github.com/rust-lang/crates.io-index"
dependencies = [
"cfg-if 0.1.5 (registry+https://github.com/rust-lang/crates.io-index)",
"gcc 0.3.55 (registry+https://github.com/rust-lang/crates.io-index)",
"libc 0.2.43 (registry+https://github.com/rust-lang/crates.io-index)",
]

[[package]]
name = "std"
version = "0.0.0"
Expand Down Expand Up @@ -3240,6 +3256,7 @@ source = "registry+https://github.com/rust-lang/crates.io-index"
"checksum futf 0.1.4 (registry+https://github.com/rust-lang/crates.io-index)" = "7c9c1ce3fa9336301af935ab852c437817d14cd33690446569392e65170aac3b"
"checksum futures 0.1.21 (registry+https://github.com/rust-lang/crates.io-index)" = "1a70b146671de62ec8c8ed572219ca5d594d9b06c0b364d5e67b722fc559b48c"
"checksum fwdansi 1.0.1 (registry+https://github.com/rust-lang/crates.io-index)" = "34dd4c507af68d37ffef962063dfa1944ce0dd4d5b82043dbab1dabe088610c3"
"checksum gcc 0.3.55 (registry+https://github.com/rust-lang/crates.io-index)" = "8f5f3913fa0bfe7ee1fd8248b6b9f42a5af4b9d65ec2dd2c3c26132b950ecfc2"
"checksum getopts 0.2.17 (registry+https://github.com/rust-lang/crates.io-index)" = "b900c08c1939860ce8b54dc6a89e26e00c04c380fd0e09796799bd7f12861e05"
"checksum git2 0.7.5 (registry+https://github.com/rust-lang/crates.io-index)" = "591f8be1674b421644b6c030969520bc3fa12114d2eb467471982ed3e9584e71"
"checksum git2-curl 0.8.2 (registry+https://github.com/rust-lang/crates.io-index)" = "0173e317f8ba21f3fff0f71549fead5e42e67961dbd402bf69f42775f3cc78b4"
Expand Down Expand Up @@ -3379,6 +3396,7 @@ source = "registry+https://github.com/rust-lang/crates.io-index"
"checksum smallvec 0.6.5 (registry+https://github.com/rust-lang/crates.io-index)" = "153ffa32fd170e9944f7e0838edf824a754ec4c1fc64746fcc9fe1f8fa602e5d"
"checksum socket2 0.3.8 (registry+https://github.com/rust-lang/crates.io-index)" = "c4d11a52082057d87cb5caa31ad812f4504b97ab44732cd8359df2e9ff9f48e7"
"checksum stable_deref_trait 1.1.0 (registry+https://github.com/rust-lang/crates.io-index)" = "ffbc596e092fe5f598b12ef46cc03754085ac2f4d8c739ad61c4ae266cc3b3fa"
"checksum stacker 0.1.3 (registry+https://github.com/rust-lang/crates.io-index)" = "82c150485b78a81ed189dbdd1947397344bc296b86f7fcc7ca3cdae8bfe882e0"
"checksum string_cache 0.7.3 (registry+https://github.com/rust-lang/crates.io-index)" = "25d70109977172b127fe834e5449e5ab1740b9ba49fa18a2020f509174f25423"
"checksum string_cache_codegen 0.4.1 (registry+https://github.com/rust-lang/crates.io-index)" = "35293b05cf1494e8ddd042a7df6756bf18d07f42d234f32e71dce8a7aabb0191"
"checksum string_cache_shared 0.3.0 (registry+https://github.com/rust-lang/crates.io-index)" = "b1884d1bc09741d466d9b14e6d37ac89d6909cbcac41dd9ae982d4d063bbedfc"
Expand Down
1 change: 1 addition & 0 deletions src/librustc/Cargo.toml
Original file line number Diff line number Diff line change
Expand Up @@ -34,6 +34,7 @@ byteorder = { version = "1.1", features = ["i128"]}
chalk-engine = { version = "0.8.0", default-features=false }
rustc_fs_util = { path = "../librustc_fs_util" }
smallvec = { version = "0.6.5", features = ["union"] }
stacker = "0.1.3"

# Note that these dependencies are a lie, they're just here to get linkage to
# work.
Expand Down
257 changes: 130 additions & 127 deletions src/librustc/hir/lowering.rs
Original file line number Diff line number Diff line change
Expand Up @@ -50,6 +50,7 @@ use hir::GenericArg;
use lint::builtin::{self, PARENTHESIZED_PARAMS_IN_TYPES_AND_MODULES,
ELIDED_LIFETIMES_IN_PATHS};
use middle::cstore::CrateStore;
use middle::recursion_limit::ensure_sufficient_stack;
use rustc_data_structures::fx::FxHashSet;
use rustc_data_structures::indexed_vec::IndexVec;
use rustc_data_structures::thin_vec::ThinVec;
Expand Down Expand Up @@ -3659,8 +3660,8 @@ impl<'a> LoweringContext<'a> {
})
}

fn lower_expr(&mut self, e: &Expr) -> hir::Expr {
let kind = match e.node {
fn lower_expr_kind(&mut self, e: &Expr) -> hir::ExprKind {
match e.node {
ExprKind::Box(ref inner) => hir::ExprKind::Box(P(self.lower_expr(inner))),
ExprKind::ObsoleteInPlace(..) => {
self.sess.abort_if_errors();
Expand Down Expand Up @@ -3959,19 +3960,11 @@ impl<'a> LoweringContext<'a> {
let struct_path = self.std_path(e.span, &struct_path, None, is_unit);
let struct_path = hir::QPath::Resolved(None, P(struct_path));

let LoweredNodeId { node_id, hir_id } = self.lower_node_id(e.id);

return hir::Expr {
id: node_id,
hir_id,
node: if is_unit {
if is_unit {
hir::ExprKind::Path(struct_path)
} else {
hir::ExprKind::Struct(struct_path, fields, None)
},
span: e.span,
attrs: e.attrs.clone(),
};
}
}
ExprKind::Path(ref qself, ref path) => {
let qpath = self.lower_qpath(
Expand Down Expand Up @@ -4050,18 +4043,7 @@ impl<'a> LoweringContext<'a> {
fields.iter().map(|x| self.lower_field(x)).collect(),
maybe_expr.as_ref().map(|x| P(self.lower_expr(x))),
),
ExprKind::Paren(ref ex) => {
let mut ex = self.lower_expr(ex);
// include parens in span, but only if it is a super-span.
if e.span.contains(ex.span) {
ex.span = e.span;
}
// merge attributes into the inner expression.
let mut attrs = e.attrs.clone();
attrs.extend::<Vec<_>>(ex.attrs.into());
ex.attrs = attrs;
return ex;
}
ExprKind::Paren(_) => bug!("parens are handled in `lower_expr`"),

ExprKind::Yield(ref opt_expr) => {
self.is_generator = true;
Expand Down Expand Up @@ -4173,6 +4155,128 @@ impl<'a> LoweringContext<'a> {
loop_expr
}

ExprKind::ForLoop(..) => bug!(),

// Desugar ExprKind::Try
// From: `<expr>?`
ExprKind::Try(ref sub_expr) => {
// to:
//
// match Try::into_result(<expr>) {
// Ok(val) => #[allow(unreachable_code)] val,
// Err(err) => #[allow(unreachable_code)]
// // If there is an enclosing `catch {...}`
// break 'catch_target Try::from_error(From::from(err)),
// // Otherwise
// return Try::from_error(From::from(err)),
// }

let unstable_span =
self.allow_internal_unstable(CompilerDesugaringKind::QuestionMark, e.span);

// Try::into_result(<expr>)
let discr = {
// expand <expr>
let sub_expr = self.lower_expr(sub_expr);

let path = &["ops", "Try", "into_result"];
let path = P(self.expr_std_path(
unstable_span, path, None, ThinVec::new()));
P(self.expr_call(e.span, path, hir_vec![sub_expr]))
};

// #[allow(unreachable_code)]
let attr = {
// allow(unreachable_code)
let allow = {
let allow_ident = Ident::from_str("allow").with_span_pos(e.span);
let uc_ident = Ident::from_str("unreachable_code").with_span_pos(e.span);
let uc_nested = attr::mk_nested_word_item(uc_ident);
attr::mk_list_item(e.span, allow_ident, vec![uc_nested])
};
attr::mk_spanned_attr_outer(e.span, attr::mk_attr_id(), allow)
};
let attrs = vec![attr];

// Ok(val) => #[allow(unreachable_code)] val,
let ok_arm = {
let val_ident = self.str_to_ident("val");
let val_pat = self.pat_ident(e.span, val_ident);
let val_expr = P(self.expr_ident_with_attrs(
e.span,
val_ident,
val_pat.id,
ThinVec::from(attrs.clone()),
));
let ok_pat = self.pat_ok(e.span, val_pat);

self.arm(hir_vec![ok_pat], val_expr)
};

// Err(err) => #[allow(unreachable_code)]
// return Try::from_error(From::from(err)),
let err_arm = {
let err_ident = self.str_to_ident("err");
let err_local = self.pat_ident(e.span, err_ident);
let from_expr = {
let path = &["convert", "From", "from"];
let from = P(self.expr_std_path(
e.span, path, None, ThinVec::new()));
let err_expr = self.expr_ident(e.span, err_ident, err_local.id);

self.expr_call(e.span, from, hir_vec![err_expr])
};
let from_err_expr =
self.wrap_in_try_constructor("from_error", from_expr, unstable_span);
let thin_attrs = ThinVec::from(attrs);
let catch_scope = self.catch_scopes.last().map(|x| *x);
let ret_expr = if let Some(catch_node) = catch_scope {
P(self.expr(
e.span,
hir::ExprKind::Break(
hir::Destination {
label: None,
target_id: Ok(catch_node),
},
Some(from_err_expr),
),
thin_attrs,
))
} else {
P(self.expr(e.span, hir::ExprKind::Ret(Some(from_err_expr)), thin_attrs))
};

let err_pat = self.pat_err(e.span, err_local);
self.arm(hir_vec![err_pat], ret_expr)
};

hir::ExprKind::Match(
discr,
hir_vec![err_arm, ok_arm],
hir::MatchSource::TryDesugar,
)
}

ExprKind::Mac(_) => panic!("Shouldn't exist here"),
}
}


fn lower_expr(&mut self, e: &Expr) -> hir::Expr {
// parens and for loops have some custom desugaring going on where the node ids aren't
// just lowered from the expression
let kind = match e.node {
ExprKind::Paren(ref ex) => {
let mut ex = ensure_sufficient_stack(|| self.lower_expr(ex));
// include parens in span, but only if it is a super-span.
if e.span.contains(ex.span) {
ex.span = e.span;
}
// merge attributes into the inner expression.
ex.attrs.extend(e.attrs.iter().cloned());
return ex;
},

// Desugar ExprForLoop
// From: `[opt_ident]: for <pat> in <head> <body>`
ExprKind::ForLoop(ref pat, ref head, ref body, opt_label) => {
Expand Down Expand Up @@ -4341,109 +4445,8 @@ impl<'a> LoweringContext<'a> {
let block = P(self.block_all(e.span, hir_vec![let_stmt], Some(result)));
// add the attributes to the outer returned expr node
return self.expr_block(block, e.attrs.clone());
}

// Desugar ExprKind::Try
// From: `<expr>?`
ExprKind::Try(ref sub_expr) => {
// to:
//
// match Try::into_result(<expr>) {
// Ok(val) => #[allow(unreachable_code)] val,
// Err(err) => #[allow(unreachable_code)]
// // If there is an enclosing `catch {...}`
// break 'catch_target Try::from_error(From::from(err)),
// // Otherwise
// return Try::from_error(From::from(err)),
// }

let unstable_span =
self.allow_internal_unstable(CompilerDesugaringKind::QuestionMark, e.span);

// Try::into_result(<expr>)
let discr = {
// expand <expr>
let sub_expr = self.lower_expr(sub_expr);

let path = &["ops", "Try", "into_result"];
let path = P(self.expr_std_path(
unstable_span, path, None, ThinVec::new()));
P(self.expr_call(e.span, path, hir_vec![sub_expr]))
};

// #[allow(unreachable_code)]
let attr = {
// allow(unreachable_code)
let allow = {
let allow_ident = Ident::from_str("allow").with_span_pos(e.span);
let uc_ident = Ident::from_str("unreachable_code").with_span_pos(e.span);
let uc_nested = attr::mk_nested_word_item(uc_ident);
attr::mk_list_item(e.span, allow_ident, vec![uc_nested])
};
attr::mk_spanned_attr_outer(e.span, attr::mk_attr_id(), allow)
};
let attrs = vec![attr];

// Ok(val) => #[allow(unreachable_code)] val,
let ok_arm = {
let val_ident = self.str_to_ident("val");
let val_pat = self.pat_ident(e.span, val_ident);
let val_expr = P(self.expr_ident_with_attrs(
e.span,
val_ident,
val_pat.id,
ThinVec::from(attrs.clone()),
));
let ok_pat = self.pat_ok(e.span, val_pat);

self.arm(hir_vec![ok_pat], val_expr)
};

// Err(err) => #[allow(unreachable_code)]
// return Try::from_error(From::from(err)),
let err_arm = {
let err_ident = self.str_to_ident("err");
let err_local = self.pat_ident(e.span, err_ident);
let from_expr = {
let path = &["convert", "From", "from"];
let from = P(self.expr_std_path(
e.span, path, None, ThinVec::new()));
let err_expr = self.expr_ident(e.span, err_ident, err_local.id);

self.expr_call(e.span, from, hir_vec![err_expr])
};
let from_err_expr =
self.wrap_in_try_constructor("from_error", from_expr, unstable_span);
let thin_attrs = ThinVec::from(attrs);
let catch_scope = self.catch_scopes.last().map(|x| *x);
let ret_expr = if let Some(catch_node) = catch_scope {
P(self.expr(
e.span,
hir::ExprKind::Break(
hir::Destination {
label: None,
target_id: Ok(catch_node),
},
Some(from_err_expr),
),
thin_attrs,
))
} else {
P(self.expr(e.span, hir::ExprKind::Ret(Some(from_err_expr)), thin_attrs))
};

let err_pat = self.pat_err(e.span, err_local);
self.arm(hir_vec![err_pat], ret_expr)
};

hir::ExprKind::Match(
discr,
hir_vec![err_arm, ok_arm],
hir::MatchSource::TryDesugar,
)
}

ExprKind::Mac(_) => panic!("Shouldn't exist here"),
},
_ => ensure_sufficient_stack(|| self.lower_expr_kind(e)),
};

let LoweredNodeId { node_id, hir_id } = self.lower_node_id(e.id);
Expand Down
1 change: 1 addition & 0 deletions src/librustc/lib.rs
Original file line number Diff line number Diff line change
Expand Up @@ -106,6 +106,7 @@ extern crate serialize as rustc_serialize; // used by deriving
extern crate rustc_apfloat;
extern crate byteorder;
extern crate backtrace;
extern crate stacker;

#[macro_use]
extern crate smallvec;
Expand Down
Loading

0 comments on commit 2b10b3d

Please sign in to comment.