Skip to content

Commit

Permalink
Rollup merge of rust-lang#55617 - oli-obk:stacker, r=oli-obk,nagisa
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 rust-lang#55471
fixes rust-lang#41884
fixes rust-lang#40161
fixes rust-lang#34844
fixes rust-lang#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
Centril committed Mar 18, 2020
2 parents f509b26 + fb10e5c commit d6e0e11
Show file tree
Hide file tree
Showing 15 changed files with 248 additions and 156 deletions.
23 changes: 23 additions & 0 deletions Cargo.lock
Original file line number Diff line number Diff line change
Expand Up @@ -2579,6 +2579,15 @@ dependencies = [
"core",
]

[[package]]
name = "psm"
version = "0.1.6"
source = "registry+https://github.com/rust-lang/crates.io-index"
checksum = "b14fc68b454f875abc8354c2555e1d56596f74833ddc0f77f87f4871ed6a30e0"
dependencies = [
"cc",
]

[[package]]
name = "publicsuffix"
version = "1.5.3"
Expand Down Expand Up @@ -3122,6 +3131,7 @@ dependencies = [
"scoped-tls",
"serialize",
"smallvec 1.0.0",
"stacker",
]

[[package]]
Expand Down Expand Up @@ -4541,6 +4551,19 @@ version = "1.1.0"
source = "registry+https://github.com/rust-lang/crates.io-index"
checksum = "ffbc596e092fe5f598b12ef46cc03754085ac2f4d8c739ad61c4ae266cc3b3fa"

[[package]]
name = "stacker"
version = "0.1.6"
source = "registry+https://github.com/rust-lang/crates.io-index"
checksum = "d96fc4f13a0ac088e9a3cd9af1cc8c5cc1ab5deb2145cef661267dfc9c542f8a"
dependencies = [
"cc",
"cfg-if",
"libc",
"psm",
"winapi 0.3.8",
]

[[package]]
name = "std"
version = "0.0.0"
Expand Down
1 change: 1 addition & 0 deletions src/librustc/Cargo.toml
Original file line number Diff line number Diff line change
Expand Up @@ -36,3 +36,4 @@ byteorder = { version = "1.3" }
smallvec = { version = "1.0", features = ["union", "may_dangle"] }
measureme = "0.7.1"
rustc_session = { path = "../librustc_session" }
stacker = "0.1.6"
18 changes: 18 additions & 0 deletions src/librustc/middle/limits.rs
Original file line number Diff line number Diff line change
Expand Up @@ -13,6 +13,24 @@ use rustc_span::symbol::{sym, Symbol};

use rustc_data_structures::sync::Once;

// 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)
}

pub fn update_limits(sess: &Session, krate: &ast::Crate) {
update_limit(sess, krate, &sess.recursion_limit, sym::recursion_limit, 128);
update_limit(sess, krate, &sess.type_length_limit, sym::type_length_limit, 1048576);
Expand Down
5 changes: 4 additions & 1 deletion src/librustc/ty/inhabitedness/mod.rs
Original file line number Diff line number Diff line change
@@ -1,5 +1,6 @@
pub use self::def_id_forest::DefIdForest;

use crate::middle::limits::ensure_sufficient_stack;
use crate::ty;
use crate::ty::context::TyCtxt;
use crate::ty::TyKind::*;
Expand Down Expand Up @@ -196,7 +197,9 @@ impl<'tcx> TyS<'tcx> {
/// Calculates the forest of `DefId`s from which this type is visibly uninhabited.
fn uninhabited_from(&self, tcx: TyCtxt<'tcx>, param_env: ty::ParamEnv<'tcx>) -> DefIdForest {
match self.kind {
Adt(def, substs) => def.uninhabited_from(tcx, substs, param_env),
Adt(def, substs) => {
ensure_sufficient_stack(|| def.uninhabited_from(tcx, substs, param_env))
}

Never => DefIdForest::full(tcx),

Expand Down
4 changes: 3 additions & 1 deletion src/librustc/ty/query/plumbing.rs
Original file line number Diff line number Diff line change
Expand Up @@ -321,7 +321,9 @@ impl<'tcx> TyCtxt<'tcx> {
};

// Use the `ImplicitCtxt` while we execute the query.
tls::enter_context(&new_icx, |_| compute(self))
tls::enter_context(&new_icx, |_| {
crate::middle::limits::ensure_sufficient_stack(|| compute(self))
})
})
}

Expand Down
66 changes: 37 additions & 29 deletions src/librustc_ast_lowering/expr.rs
Original file line number Diff line number Diff line change
@@ -1,6 +1,7 @@
use super::{ImplTraitContext, LoweringContext, ParamMode, ParenthesizedGenericArgs};

use rustc::bug;
use rustc::middle::limits::ensure_sufficient_stack;
use rustc::{bug, span_bug};
use rustc_ast::ast::*;
use rustc_ast::attr;
use rustc_ast::ptr::P as AstP;
Expand All @@ -22,6 +23,37 @@ impl<'hir> LoweringContext<'_, 'hir> {

pub(super) fn lower_expr_mut(&mut self, e: &Expr) -> hir::Expr<'hir> {
let kind = match e.kind {
ExprKind::Paren(ref ex) => {
let mut ex = self.lower_expr_mut(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;
}

// Desugar `ExprForLoop`
// from: `[opt_ident]: for <pat> in <head> <body>`
ExprKind::ForLoop(ref pat, ref head, ref body, opt_label) => {
return self.lower_expr_for(e, pat, head, body, opt_label);
}
_ => ensure_sufficient_stack(|| self.lower_expr_kind(e)),
};

hir::Expr {
hir_id: self.lower_node_id(e.id),
kind,
span: e.span,
attrs: e.attrs.iter().map(|a| self.lower_attr(a)).collect::<Vec<_>>().into(),
}
}

fn lower_expr_kind(&mut self, e: &Expr) -> hir::ExprKind<'hir> {
match e.kind {
ExprKind::Box(ref inner) => hir::ExprKind::Box(self.lower_expr(inner)),
ExprKind::Array(ref exprs) => hir::ExprKind::Array(self.lower_exprs(exprs)),
ExprKind::Repeat(ref expr, ref count) => {
Expand Down Expand Up @@ -175,37 +207,13 @@ impl<'hir> LoweringContext<'_, 'hir> {
maybe_expr,
)
}
ExprKind::Paren(ref ex) => {
let mut ex = self.lower_expr_mut(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::Yield(ref opt_expr) => self.lower_expr_yield(e.span, opt_expr.as_deref()),

ExprKind::Err => hir::ExprKind::Err,

// Desugar `ExprForLoop`
// from: `[opt_ident]: for <pat> in <head> <body>`
ExprKind::ForLoop(ref pat, ref head, ref body, opt_label) => {
return self.lower_expr_for(e, pat, head, body, opt_label);
}
ExprKind::Try(ref sub_expr) => self.lower_expr_try(e.span, sub_expr),
ExprKind::MacCall(_) => panic!("Shouldn't exist here"),
};

hir::Expr {
hir_id: self.lower_node_id(e.id),
kind,
span: e.span,
attrs: e.attrs.iter().map(|a| self.lower_attr(a)).collect::<Vec<_>>().into(),
ExprKind::ForLoop(..) | ExprKind::Paren(_) => {
span_bug!(e.span, "Should have been handled by `lower_expr`")
}
ExprKind::MacCall(_) => span_bug!(e.span, "Shouldn't exist here"),
}
}

Expand Down
19 changes: 14 additions & 5 deletions src/librustc_ast_lowering/pat.rs
Original file line number Diff line number Diff line change
@@ -1,5 +1,6 @@
use super::{ImplTraitContext, LoweringContext, ParamMode};

use rustc::{middle::limits::ensure_sufficient_stack, span_bug};
use rustc_ast::ast::*;
use rustc_ast::ptr::P;
use rustc_hir as hir;
Expand All @@ -9,6 +10,16 @@ use rustc_span::{source_map::Spanned, Span};
impl<'a, 'hir> LoweringContext<'a, 'hir> {
crate fn lower_pat(&mut self, p: &Pat) -> &'hir hir::Pat<'hir> {
let node = match p.kind {
// FIXME: consider not using recursion to lower this.
PatKind::Paren(ref inner) => return self.lower_pat(inner),
_ => ensure_sufficient_stack(|| self.lower_pat_kind(p)),
};

self.pat_with_node_id_of(p, node)
}

fn lower_pat_kind(&mut self, p: &Pat) -> hir::PatKind<'hir> {
match p.kind {
PatKind::Wild => hir::PatKind::Wild,
PatKind::Ident(ref binding_mode, ident, ref sub) => {
let lower_sub = |this: &mut Self| sub.as_ref().map(|s| this.lower_pat(&*s));
Expand Down Expand Up @@ -74,11 +85,9 @@ impl<'a, 'hir> LoweringContext<'a, 'hir> {
// If we reach here the `..` pattern is not semantically allowed.
self.ban_illegal_rest_pat(p.span)
}
PatKind::Paren(ref inner) => return self.lower_pat(inner),
PatKind::MacCall(_) => panic!("Shouldn't exist here"),
};

self.pat_with_node_id_of(p, node)
PatKind::Paren(_) => span_bug!(p.span, "Should have been handled by `lower_pat`"),
PatKind::MacCall(_) => span_bug!(p.span, "Shouldn't exist here"),
}
}

fn lower_pat_tuple(
Expand Down
9 changes: 1 addition & 8 deletions src/librustc_interface/util.rs
Original file line number Diff line number Diff line change
Expand Up @@ -83,14 +83,7 @@ pub fn create_session(
(Lrc::new(sess), Lrc::new(codegen_backend), source_map)
}

// Temporarily have stack size set to 32MB to deal with various crates with long method
// chains or deep syntax trees, except when on Haiku.
// FIXME(oli-obk): get https://github.com/rust-lang/rust/pull/55617 the finish line
#[cfg(not(target_os = "haiku"))]
const STACK_SIZE: usize = 32 * 1024 * 1024;

#[cfg(target_os = "haiku")]
const STACK_SIZE: usize = 16 * 1024 * 1024;
const STACK_SIZE: usize = 8 * 1024 * 1024;

fn get_stack_size() -> Option<usize> {
// FIXME: Hacks on hacks. If the env is trying to override the stack size
Expand Down
8 changes: 6 additions & 2 deletions src/librustc_mir/monomorphize/collector.rs
Original file line number Diff line number Diff line change
Expand Up @@ -368,7 +368,9 @@ fn collect_items_rec<'tcx>(
recursion_depth_reset = Some(check_recursion_limit(tcx, instance, recursion_depths));
check_type_length_limit(tcx, instance);

collect_neighbours(tcx, instance, &mut neighbors);
rustc::middle::limits::ensure_sufficient_stack(|| {
collect_neighbours(tcx, instance, &mut neighbors);
});
}
MonoItem::GlobalAsm(..) => {
recursion_depth_reset = None;
Expand Down Expand Up @@ -1134,7 +1136,9 @@ fn collect_miri<'tcx>(tcx: TyCtxt<'tcx>, alloc_id: AllocId, output: &mut Vec<Mon
Some(GlobalAlloc::Memory(alloc)) => {
trace!("collecting {:?} with {:#?}", alloc_id, alloc);
for &((), inner) in alloc.relocations().values() {
collect_miri(tcx, inner, output);
rustc::middle::limits::ensure_sufficient_stack(|| {
collect_miri(tcx, inner, output);
});
}
}
Some(GlobalAlloc::Function(fn_instance)) => {
Expand Down
8 changes: 6 additions & 2 deletions src/librustc_mir_build/build/expr/as_temp.rs
Original file line number Diff line number Diff line change
Expand Up @@ -3,7 +3,7 @@
use crate::build::scope::DropKind;
use crate::build::{BlockAnd, BlockAndExtension, Builder};
use crate::hair::*;
use rustc::middle::region;
use rustc::middle::{region, limits::ensure_sufficient_stack};
use rustc::mir::*;
use rustc_hir as hir;
use rustc_span::symbol::sym;
Expand All @@ -22,7 +22,11 @@ impl<'a, 'tcx> Builder<'a, 'tcx> {
M: Mirror<'tcx, Output = Expr<'tcx>>,
{
let expr = self.hir.mirror(expr);
self.expr_as_temp(block, temp_lifetime, expr, mutability)
//
// this is the only place in mir building that we need to truly need to worry about
// infinite recursion. Everything else does recurse, too, but it always gets broken up
// at some point by inserting an intermediate temporary
ensure_sufficient_stack(|| self.expr_as_temp(block, temp_lifetime, expr, mutability))
}

fn expr_as_temp(
Expand Down
3 changes: 2 additions & 1 deletion src/librustc_trait_selection/traits/project.rs
Original file line number Diff line number Diff line change
Expand Up @@ -17,6 +17,7 @@ use super::{VtableClosureData, VtableFnPointerData, VtableGeneratorData, VtableI
use crate::infer::type_variable::{TypeVariableOrigin, TypeVariableOriginKind};
use crate::infer::{InferCtxt, InferOk, LateBoundRegionConversionTime};
use crate::traits::error_reporting::InferCtxtExt;
use rustc::middle::limits::ensure_sufficient_stack;
use rustc::ty::fold::{TypeFoldable, TypeFolder};
use rustc::ty::subst::{InternalSubsts, Subst};
use rustc::ty::{self, ToPolyTraitRef, ToPredicate, Ty, TyCtxt, WithConstness};
Expand Down Expand Up @@ -261,7 +262,7 @@ where
{
debug!("normalize_with_depth(depth={}, value={:?})", depth, value);
let mut normalizer = AssocTypeNormalizer::new(selcx, param_env, cause, depth, obligations);
let result = normalizer.fold(value);
let result = ensure_sufficient_stack(|| normalizer.fold(value));
debug!(
"normalize_with_depth: depth={} result={:?} with {} obligations",
depth,
Expand Down
3 changes: 2 additions & 1 deletion src/librustc_trait_selection/traits/query/normalize.rs
Original file line number Diff line number Diff line change
Expand Up @@ -7,6 +7,7 @@ use crate::infer::canonical::OriginalQueryValues;
use crate::infer::{InferCtxt, InferOk};
use crate::traits::error_reporting::InferCtxtExt;
use crate::traits::{Obligation, ObligationCause, PredicateObligation, Reveal};
use rustc::middle::limits::ensure_sufficient_stack;
use rustc::ty::fold::{TypeFoldable, TypeFolder};
use rustc::ty::subst::Subst;
use rustc::ty::{self, Ty, TyCtxt};
Expand Down Expand Up @@ -120,7 +121,7 @@ impl<'cx, 'tcx> TypeFolder<'tcx> for QueryNormalizer<'cx, 'tcx> {
ty
);
}
let folded_ty = self.fold_ty(concrete_ty);
let folded_ty = ensure_sufficient_stack(|| self.fold_ty(concrete_ty));
self.anon_depth -= 1;
folded_ty
}
Expand Down
Loading

0 comments on commit d6e0e11

Please sign in to comment.