Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

Sign up for GitHub

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Jump to bottom

Stack overflow in rustc, regression from 1.44 #74564

Closed
alexcrichton opened this issue Jul 20, 2020 · 6 comments · Fixed by #74708
Closed

Stack overflow in rustc, regression from 1.44 #74564

alexcrichton opened this issue Jul 20, 2020 · 6 comments · Fixed by #74708
Labels
C-bug Category: This is a bug. E-easy Call for participation: Easy difficulty. Experience needed to fix: Not much. Good first issue. I-crash Issue: The compiler crashes (SIGSEGV, SIGABRT, etc). Use I-ICE instead when the compiler panics. ICEBreaker-Cleanup-Crew Helping to "clean up" bugs with minimal examples and bisections P-high High priority regression-from-stable-to-stable Performance or correctness regression from one stable version to another. T-compiler Relevant to the compiler team, which will review and decide on the PR/issue.

Comments

@alexcrichton
Copy link
Member

First reported to me at rustwasm/wasm-bindgen#2244 I believe the stack overflow is in rustc itself. This code when compiled yields:

$ rustc +1.44.0 foo.rs
$ rustc +stable foo.rs

thread 'rustc' has overflowed its stack
fatal runtime error: stack overflow
zsh: abort (core dumped)  rustc +stable foo.rs
$ rustc +nightly foo.rs

thread 'rustc' has overflowed its stack
fatal runtime error: stack overflow
zsh: abort (core dumped)  rustc +nightly foo.rs

where nightly today is:

$ rustc +nightly -vV
rustc 1.47.0-nightly (d7f945163 2020-07-19)
binary: rustc
commit-hash: d7f94516345a36ddfcd68cbdf1df835d356795c3
commit-date: 2020-07-19
host: x86_64-unknown-linux-gnu
release: 1.47.0-nightly
LLVM version: 10.0
@alexcrichton alexcrichton mentioned this issue Jul 20, 2020
Closed
@jonas-schievink jonas-schievink added C-bug Category: This is a bug. I-crash Issue: The compiler crashes (SIGSEGV, SIGABRT, etc). Use I-ICE instead when the compiler panics. regression-from-stable-to-stable Performance or correctness regression from one stable version to another. T-compiler Relevant to the compiler team, which will review and decide on the PR/issue. labels Jul 20, 2020
@rustbot rustbot added the I-prioritize Issue: Indicates that prioritization has been requested for this issue. label Jul 20, 2020
@spastorino
Copy link
Member

Assigning P-high as discussed as part of the Prioritization Working Group procedure and removing I-prioritize.

@spastorino spastorino added P-high High priority and removed I-prioritize Issue: Indicates that prioritization has been requested for this issue. labels Jul 22, 2020
@spastorino
Copy link
Member

It would be nice to bisect this one if possible ...

@rustbot ping cleanup

@rustbot rustbot added the ICEBreaker-Cleanup-Crew Helping to "clean up" bugs with minimal examples and bisections label Jul 22, 2020
@rustbot
Copy link
Collaborator

rustbot commented Jul 22, 2020

Hey Cleanup Crew ICE-breakers! This bug has been identified as a good
"Cleanup ICE-breaking candidate". In case it's useful, here are some
instructions for tackling these sorts of bugs. Maybe take a look?
Thanks! <3

cc @AminArria @camelid @chrissimpkins @contrun @DutchGhost @elshize @ethanboxx @h-michael @HallerPatrick @hdhoang @hellow554 @imtsuki @kanru @KarlK90 @LeSeulArtichaut @MAdrianMattocks @matheus-consoli @mental32 @nmccarty @Noah-Kennedy @pard68 @PeytonT @pierreN @Redblueflame @RobbieClarken @RobertoSnap @robjtede @SarthakSingh31 @senden9 @shekohex @sinato @spastorino @turboladen @woshilapin @yerke

@kanru
Copy link
Contributor

kanru commented Jul 22, 2020

searched nightlies: from nightly-2020-05-05 to nightly-2020-07-22
regressed nightly: nightly-2020-05-08
searched commits: from 1836e3b to a08c473
regressed commit: 97f3eee

bisected with cargo-bisect-rustc v0.5.1

Host triple: x86_64-unknown-linux-gnu
Reproduce with:

cargo bisect-rustc -- check

cc @oli-obk

@oli-obk
Copy link
Contributor

oli-obk commented Jul 23, 2020

cc @nagisa

for anyone wanting to fix this, these kind of bugs are very easy to fix. You run rustc in gdb until you get a stack overflow, then you look at the backtrace and look for recursion in there. Then you break the recursion overflow by inserting a use of https://doc.rust-lang.org/nightly/nightly-rustc/rustc_data_structures/stack/fn.ensure_sufficient_stack.html at the recursive call site.

@oli-obk oli-obk added the E-easy Call for participation: Easy difficulty. Experience needed to fix: Not much. Good first issue. label Jul 23, 2020
@kanru
Copy link
Contributor

kanru commented Jul 23, 2020

That sounds a good first bug to me. I'll give it a shot.

@kanru kanru mentioned this issue Jul 24, 2020
Merged
Dylan-DPC-zz pushed a commit to Dylan-DPC-zz/rust that referenced this issue Jul 24, 2020
@Dylan-DPC
Rollup merge of rust-lang#74708 - kanru:issue-74564, r=davidtwco
bc36d69 
Ensure stack when type checking and building MIR for large if expressions

Fixes rust-lang#74564
@bors bors closed this as completed in c709862 Jul 27, 2020
@wtfsck wtfsck mentioned this issue Jun 8, 2022
Closed
@uweigand uweigand mentioned this issue Dec 6, 2022
Closed
8 tasks
uweigand added a commit to uweigand/rust that referenced this issue Dec 19, 2022
@uweigand
Fix stack overflow in recursive AST walk in early lint
6bb2bda 
The src/test/ui/issues/issue-74564-if-expr-stack-overflow.rs test case
added to verify rust-lang#74564 still
crashes with a stack overflow on s390x-ibm-linux.

Symptom is a very deep recursion in compiler/rustc_lint/src/early.rs:
    fn visit_expr(&mut self, e: &'a ast::Expr) {
        self.with_lint_attrs(e.id, &e.attrs, |cx| {
            lint_callback!(cx, check_expr, e);
            ast_visit::walk_expr(cx, e);
        })
    }
(where walk_expr recursively calls back into visit_expr).  The crash
happens at a nesting depth of over 17000 stack frames when using the
default 8 MB stack size on s390x.

This patch fixes the problem by adding a ensure_sufficient_stack
call to the with_lint_attrs routine (which also should take care
of all the other mutually recursive visitors here).

Fixes part of rust-lang#105383.
@uweigand uweigand mentioned this issue Dec 19, 2022
Merged
bors added a commit to rust-lang-ci/rust that referenced this issue Jan 11, 2023
@bors
Auto merge of rust-lang#105919 - uweigand:s390x-stack-overflow, r=Nil…
8ecaad8 
…strieb

Fix stack overflow in recursive AST walk in early lint

The src/test/ui/issues/issue-74564-if-expr-stack-overflow.rs test case added to verify rust-lang#74564 still crashes with a stack overflow on s390x-ibm-linux.

Symptom is a very deep recursion in compiler/rustc_lint/src/early.rs:
    fn visit_expr(&mut self, e: &'a ast::Expr) {
        self.with_lint_attrs(e.id, &e.attrs, |cx| {
            lint_callback!(cx, check_expr, e);
            ast_visit::walk_expr(cx, e);
        })
    }
(where walk_expr recursively calls back into visit_expr).  The crash happens at a nesting depth of over 17000 stack frames when using the default 8 MB stack size on s390x.

This patch fixes the problem by adding a ensure_sufficient_stack call to the with_lint_attrs routine (which also should take care of all the other mutually recursive visitors here).

Fixes part of rust-lang#105383.
RalfJung pushed a commit to RalfJung/miri that referenced this issue Jan 13, 2023
@bors
Auto merge of #105919 - uweigand:s390x-stack-overflow, r=Nilstrieb
32ff9ab 
Fix stack overflow in recursive AST walk in early lint

The src/test/ui/issues/issue-74564-if-expr-stack-overflow.rs test case added to verify rust-lang/rust#74564 still crashes with a stack overflow on s390x-ibm-linux.

Symptom is a very deep recursion in compiler/rustc_lint/src/early.rs:
    fn visit_expr(&mut self, e: &'a ast::Expr) {
        self.with_lint_attrs(e.id, &e.attrs, |cx| {
            lint_callback!(cx, check_expr, e);
            ast_visit::walk_expr(cx, e);
        })
    }
(where walk_expr recursively calls back into visit_expr).  The crash happens at a nesting depth of over 17000 stack frames when using the default 8 MB stack size on s390x.

This patch fixes the problem by adding a ensure_sufficient_stack call to the with_lint_attrs routine (which also should take care of all the other mutually recursive visitors here).

Fixes part of rust-lang/rust#105383.
RalfJung pushed a commit to RalfJung/rust-analyzer that referenced this issue Apr 20, 2024
@bors
Auto merge of #105919 - uweigand:s390x-stack-overflow, r=Nilstrieb
d8f9576 
Fix stack overflow in recursive AST walk in early lint

The src/test/ui/issues/issue-74564-if-expr-stack-overflow.rs test case added to verify rust-lang/rust#74564 still crashes with a stack overflow on s390x-ibm-linux.

Symptom is a very deep recursion in compiler/rustc_lint/src/early.rs:
    fn visit_expr(&mut self, e: &'a ast::Expr) {
        self.with_lint_attrs(e.id, &e.attrs, |cx| {
            lint_callback!(cx, check_expr, e);
            ast_visit::walk_expr(cx, e);
        })
    }
(where walk_expr recursively calls back into visit_expr).  The crash happens at a nesting depth of over 17000 stack frames when using the default 8 MB stack size on s390x.

This patch fixes the problem by adding a ensure_sufficient_stack call to the with_lint_attrs routine (which also should take care of all the other mutually recursive visitors here).

Fixes part of rust-lang/rust#105383.
RalfJung pushed a commit to RalfJung/rust-analyzer that referenced this issue Apr 27, 2024
@bors
Auto merge of #105919 - uweigand:s390x-stack-overflow, r=Nilstrieb
4e25d2e 
Fix stack overflow in recursive AST walk in early lint

The src/test/ui/issues/issue-74564-if-expr-stack-overflow.rs test case added to verify rust-lang/rust#74564 still crashes with a stack overflow on s390x-ibm-linux.

Symptom is a very deep recursion in compiler/rustc_lint/src/early.rs:
    fn visit_expr(&mut self, e: &'a ast::Expr) {
        self.with_lint_attrs(e.id, &e.attrs, |cx| {
            lint_callback!(cx, check_expr, e);
            ast_visit::walk_expr(cx, e);
        })
    }
(where walk_expr recursively calls back into visit_expr).  The crash happens at a nesting depth of over 17000 stack frames when using the default 8 MB stack size on s390x.

This patch fixes the problem by adding a ensure_sufficient_stack call to the with_lint_attrs routine (which also should take care of all the other mutually recursive visitors here).

Fixes part of rust-lang/rust#105383.
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Assignees
No one assigned
Labels
C-bug Category: This is a bug. E-easy Call for participation: Easy difficulty. Experience needed to fix: Not much. Good first issue. I-crash Issue: The compiler crashes (SIGSEGV, SIGABRT, etc). Use I-ICE instead when the compiler panics. ICEBreaker-Cleanup-Crew Helping to "clean up" bugs with minimal examples and bisections P-high High priority regression-from-stable-to-stable Performance or correctness regression from one stable version to another. T-compiler Relevant to the compiler team, which will review and decide on the PR/issue.
Projects
None yet
Milestone
No milestone
Development

Successfully merging a pull request may close this issue.

Ensure stack when type checking and building MIR for large if expressions kanru/rust
6 participants
@kanru @spastorino @alexcrichton @oli-obk @jonas-schievink @rustbot