Skip to content

Commit

Permalink
Auto merge of #92889 - tmiasko:unbounded-recursion, r=ecstatic-morse
Browse files Browse the repository at this point in the history
Ignore unwinding edges when checking for unconditional recursion

The unconditional recursion lint determines if all execution paths
eventually lead to a self-recursive call.

The implementation always follows unwinding edges which limits its
practical utility. For example, it would not lint function `f` because a
call to `g` might unwind. It also wouldn't lint function `h` because an
overflow check preceding the self-recursive call might unwind:

```rust
pub fn f() {
    g();
    f();
}

pub fn g() { /* ... */ }

pub fn h(a: usize) {
  h(a + 1);
}
```

To avoid the issue, assume that terminators that might continue
execution along non-unwinding edges do so.

Fixes #78474.
  • Loading branch information
bors committed Jan 27, 2022
2 parents 563250a + 10b722c commit 21b4a9c
Show file tree
Hide file tree
Showing 5 changed files with 108 additions and 7 deletions.
12 changes: 8 additions & 4 deletions compiler/rustc_mir_build/src/lints.rs
Original file line number Diff line number Diff line change
Expand Up @@ -33,6 +33,9 @@ crate fn check<'tcx>(tcx: TyCtxt<'tcx>, body: &Body<'tcx>) {
if let Some(NonRecursive) = TriColorDepthFirstSearch::new(&body).run_from_start(&mut vis) {
return;
}
if vis.reachable_recursive_calls.is_empty() {
return;
}

vis.reachable_recursive_calls.sort();

Expand Down Expand Up @@ -148,13 +151,14 @@ impl<'mir, 'tcx> TriColorVisitor<&'mir Body<'tcx>> for Search<'mir, 'tcx> {
}

fn ignore_edge(&mut self, bb: BasicBlock, target: BasicBlock) -> bool {
let terminator = self.body[bb].terminator();
if terminator.unwind() == Some(&Some(target)) && terminator.successors().count() > 1 {
return true;
}
// Don't traverse successors of recursive calls or false CFG edges.
match self.body[bb].terminator().kind {
TerminatorKind::Call { ref func, .. } => self.is_recursive_call(func),

TerminatorKind::FalseUnwind { unwind: Some(imaginary_target), .. }
| TerminatorKind::FalseEdge { imaginary_target, .. } => imaginary_target == target,

TerminatorKind::FalseEdge { imaginary_target, .. } => imaginary_target == target,
_ => false,
}
}
Expand Down
42 changes: 42 additions & 0 deletions src/test/ui/lint/lint-unconditional-recursion.rs
Original file line number Diff line number Diff line change
Expand Up @@ -149,4 +149,46 @@ pub fn panics(x: bool) {
}
}

pub fn unreachable1() {
panic!();
unreachable1(); // WARN unreachable statement
}

pub fn unreachable2() {
loop {}
unreachable2(); // WARN unreachable statement
}

pub fn drop_and_replace(mut a: Option<String>) { //~ ERROR function cannot return without recursing
a = None;
drop_and_replace(a);
}

// Calls are assumed to return normally.
pub fn call() -> String { //~ ERROR function cannot return without recursing
let s = String::new();
call();
s
}

// Arithmetic operations are assumed not to overflow.
pub fn overflow_check(a: i32, b: i32) { //~ ERROR function cannot return without recursing
let _ = a + b;
overflow_check(a, b);
}

pub struct Point {
pub x: f32,
pub y: f32,
}

impl Default for Point {
fn default() -> Self { //~ ERROR function cannot return without recursing
Point {
x: Default::default(),
..Default::default()
}
}
}

fn main() {}
46 changes: 45 additions & 1 deletion src/test/ui/lint/lint-unconditional-recursion.stderr
Original file line number Diff line number Diff line change
Expand Up @@ -153,5 +153,49 @@ LL | self.as_ref()
|
= help: a `loop` may express intention better if this is on purpose

error: aborting due to 14 previous errors
error: function cannot return without recursing
--> $DIR/lint-unconditional-recursion.rs:162:1
|
LL | pub fn drop_and_replace(mut a: Option<String>) {
| ^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^ cannot return without recursing
LL | a = None;
LL | drop_and_replace(a);
| ------------------- recursive call site
|
= help: a `loop` may express intention better if this is on purpose

error: function cannot return without recursing
--> $DIR/lint-unconditional-recursion.rs:168:1
|
LL | pub fn call() -> String {
| ^^^^^^^^^^^^^^^^^^^^^^^ cannot return without recursing
LL | let s = String::new();
LL | call();
| ------ recursive call site
|
= help: a `loop` may express intention better if this is on purpose

error: function cannot return without recursing
--> $DIR/lint-unconditional-recursion.rs:175:1
|
LL | pub fn overflow_check(a: i32, b: i32) {
| ^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^ cannot return without recursing
LL | let _ = a + b;
LL | overflow_check(a, b);
| -------------------- recursive call site
|
= help: a `loop` may express intention better if this is on purpose

error: function cannot return without recursing
--> $DIR/lint-unconditional-recursion.rs:186:5
|
LL | fn default() -> Self {
| ^^^^^^^^^^^^^^^^^^^^ cannot return without recursing
...
LL | ..Default::default()
| ------------------ recursive call site
|
= help: a `loop` may express intention better if this is on purpose

error: aborting due to 18 previous errors

2 changes: 1 addition & 1 deletion src/test/ui/recursion/issue-83150.rs
Original file line number Diff line number Diff line change
Expand Up @@ -6,6 +6,6 @@ fn main() {
func(&mut iter)
}

fn func<T: Iterator<Item = u8>>(iter: &mut T) {
fn func<T: Iterator<Item = u8>>(iter: &mut T) { //~ WARN function cannot return without recursing
func(&mut iter.map(|x| x + 1))
}
13 changes: 12 additions & 1 deletion src/test/ui/recursion/issue-83150.stderr
Original file line number Diff line number Diff line change
@@ -1,8 +1,19 @@
warning: function cannot return without recursing
--> $DIR/issue-83150.rs:9:1
|
LL | fn func<T: Iterator<Item = u8>>(iter: &mut T) {
| ^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^ cannot return without recursing
LL | func(&mut iter.map(|x| x + 1))
| ------------------------------ recursive call site
|
= note: `#[warn(unconditional_recursion)]` on by default
= help: a `loop` may express intention better if this is on purpose

error[E0275]: overflow evaluating the requirement `Map<&mut Map<&mut Map<&mut Map<&mut Map<&mut Map<&mut Map<&mut Map<&mut Map<&mut Map<&mut Map<&mut Map<&mut Map<&mut Map<&mut Map<&mut Map<&mut Map<&mut Map<&mut Map<&mut Map<&mut Map<&mut Map<&mut Map<&mut Map<&mut Map<&mut Map<&mut Map<&mut Map<&mut Map<&mut Map<&mut Map<&mut Map<&mut Map<&mut Map<&mut Map<&mut Map<&mut Map<&mut Map<&mut Map<&mut Map<&mut Map<&mut Map<&mut Map<&mut Map<&mut Map<&mut Map<&mut Map<&mut Map<&mut Map<&mut Map<&mut Map<&mut Map<&mut Map<&mut Map<&mut Map<&mut Map<&mut Map<&mut Map<&mut Map<&mut Map<&mut Map<&mut Map<&mut Map<&mut Map<&mut std::ops::Range<u8>, [closure@$DIR/issue-83150.rs:10:24: 10:33]>, [closure@$DIR/issue-83150.rs:10:24: 10:33]>, [closure@$DIR/issue-83150.rs:10:24: 10:33]>, [closure@$DIR/issue-83150.rs:10:24: 10:33]>, [closure@$DIR/issue-83150.rs:10:24: 10:33]>, [closure@$DIR/issue-83150.rs:10:24: 10:33]>, [closure@$DIR/issue-83150.rs:10:24: 10:33]>, [closure@$DIR/issue-83150.rs:10:24: 10:33]>, [closure@$DIR/issue-83150.rs:10:24: 10:33]>, [closure@$DIR/issue-83150.rs:10:24: 10:33]>, [closure@$DIR/issue-83150.rs:10:24: 10:33]>, [closure@$DIR/issue-83150.rs:10:24: 10:33]>, [closure@$DIR/issue-83150.rs:10:24: 10:33]>, [closure@$DIR/issue-83150.rs:10:24: 10:33]>, [closure@$DIR/issue-83150.rs:10:24: 10:33]>, [closure@$DIR/issue-83150.rs:10:24: 10:33]>, [closure@$DIR/issue-83150.rs:10:24: 10:33]>, [closure@$DIR/issue-83150.rs:10:24: 10:33]>, [closure@$DIR/issue-83150.rs:10:24: 10:33]>, [closure@$DIR/issue-83150.rs:10:24: 10:33]>, [closure@$DIR/issue-83150.rs:10:24: 10:33]>, [closure@$DIR/issue-83150.rs:10:24: 10:33]>, [closure@$DIR/issue-83150.rs:10:24: 10:33]>, [closure@$DIR/issue-83150.rs:10:24: 10:33]>, [closure@$DIR/issue-83150.rs:10:24: 10:33]>, [closure@$DIR/issue-83150.rs:10:24: 10:33]>, [closure@$DIR/issue-83150.rs:10:24: 10:33]>, [closure@$DIR/issue-83150.rs:10:24: 10:33]>, [closure@$DIR/issue-83150.rs:10:24: 10:33]>, [closure@$DIR/issue-83150.rs:10:24: 10:33]>, [closure@$DIR/issue-83150.rs:10:24: 10:33]>, [closure@$DIR/issue-83150.rs:10:24: 10:33]>, [closure@$DIR/issue-83150.rs:10:24: 10:33]>, [closure@$DIR/issue-83150.rs:10:24: 10:33]>, [closure@$DIR/issue-83150.rs:10:24: 10:33]>, [closure@$DIR/issue-83150.rs:10:24: 10:33]>, [closure@$DIR/issue-83150.rs:10:24: 10:33]>, [closure@$DIR/issue-83150.rs:10:24: 10:33]>, [closure@$DIR/issue-83150.rs:10:24: 10:33]>, [closure@$DIR/issue-83150.rs:10:24: 10:33]>, [closure@$DIR/issue-83150.rs:10:24: 10:33]>, [closure@$DIR/issue-83150.rs:10:24: 10:33]>, [closure@$DIR/issue-83150.rs:10:24: 10:33]>, [closure@$DIR/issue-83150.rs:10:24: 10:33]>, [closure@$DIR/issue-83150.rs:10:24: 10:33]>, [closure@$DIR/issue-83150.rs:10:24: 10:33]>, [closure@$DIR/issue-83150.rs:10:24: 10:33]>, [closure@$DIR/issue-83150.rs:10:24: 10:33]>, [closure@$DIR/issue-83150.rs:10:24: 10:33]>, [closure@$DIR/issue-83150.rs:10:24: 10:33]>, [closure@$DIR/issue-83150.rs:10:24: 10:33]>, [closure@$DIR/issue-83150.rs:10:24: 10:33]>, [closure@$DIR/issue-83150.rs:10:24: 10:33]>, [closure@$DIR/issue-83150.rs:10:24: 10:33]>, [closure@$DIR/issue-83150.rs:10:24: 10:33]>, [closure@$DIR/issue-83150.rs:10:24: 10:33]>, [closure@$DIR/issue-83150.rs:10:24: 10:33]>, [closure@$DIR/issue-83150.rs:10:24: 10:33]>, [closure@$DIR/issue-83150.rs:10:24: 10:33]>, [closure@$DIR/issue-83150.rs:10:24: 10:33]>, [closure@$DIR/issue-83150.rs:10:24: 10:33]>, [closure@$DIR/issue-83150.rs:10:24: 10:33]>, [closure@$DIR/issue-83150.rs:10:24: 10:33]>, [closure@$DIR/issue-83150.rs:10:24: 10:33]>: Iterator`
|
= help: consider increasing the recursion limit by adding a `#![recursion_limit = "256"]` attribute to your crate (`issue_83150`)
= note: required because of the requirements on the impl of `Iterator` for `&mut Map<&mut Map<&mut Map<&mut Map<&mut Map<&mut Map<&mut Map<&mut Map<&mut Map<&mut Map<&mut Map<&mut Map<&mut Map<&mut Map<&mut Map<&mut Map<&mut Map<&mut Map<&mut Map<&mut Map<&mut Map<&mut Map<&mut Map<&mut Map<&mut Map<&mut Map<&mut Map<&mut Map<&mut Map<&mut Map<&mut Map<&mut Map<&mut Map<&mut Map<&mut Map<&mut Map<&mut Map<&mut Map<&mut Map<&mut Map<&mut Map<&mut Map<&mut Map<&mut Map<&mut Map<&mut Map<&mut Map<&mut Map<&mut Map<&mut Map<&mut Map<&mut Map<&mut Map<&mut Map<&mut Map<&mut Map<&mut Map<&mut Map<&mut Map<&mut Map<&mut Map<&mut Map<&mut Map<&mut Map<&mut std::ops::Range<u8>, [closure@$DIR/issue-83150.rs:10:24: 10:33]>, [closure@$DIR/issue-83150.rs:10:24: 10:33]>, [closure@$DIR/issue-83150.rs:10:24: 10:33]>, [closure@$DIR/issue-83150.rs:10:24: 10:33]>, [closure@$DIR/issue-83150.rs:10:24: 10:33]>, [closure@$DIR/issue-83150.rs:10:24: 10:33]>, [closure@$DIR/issue-83150.rs:10:24: 10:33]>, [closure@$DIR/issue-83150.rs:10:24: 10:33]>, [closure@$DIR/issue-83150.rs:10:24: 10:33]>, [closure@$DIR/issue-83150.rs:10:24: 10:33]>, [closure@$DIR/issue-83150.rs:10:24: 10:33]>, [closure@$DIR/issue-83150.rs:10:24: 10:33]>, [closure@$DIR/issue-83150.rs:10:24: 10:33]>, [closure@$DIR/issue-83150.rs:10:24: 10:33]>, [closure@$DIR/issue-83150.rs:10:24: 10:33]>, [closure@$DIR/issue-83150.rs:10:24: 10:33]>, [closure@$DIR/issue-83150.rs:10:24: 10:33]>, [closure@$DIR/issue-83150.rs:10:24: 10:33]>, [closure@$DIR/issue-83150.rs:10:24: 10:33]>, [closure@$DIR/issue-83150.rs:10:24: 10:33]>, [closure@$DIR/issue-83150.rs:10:24: 10:33]>, [closure@$DIR/issue-83150.rs:10:24: 10:33]>, [closure@$DIR/issue-83150.rs:10:24: 10:33]>, [closure@$DIR/issue-83150.rs:10:24: 10:33]>, [closure@$DIR/issue-83150.rs:10:24: 10:33]>, [closure@$DIR/issue-83150.rs:10:24: 10:33]>, [closure@$DIR/issue-83150.rs:10:24: 10:33]>, [closure@$DIR/issue-83150.rs:10:24: 10:33]>, [closure@$DIR/issue-83150.rs:10:24: 10:33]>, [closure@$DIR/issue-83150.rs:10:24: 10:33]>, [closure@$DIR/issue-83150.rs:10:24: 10:33]>, [closure@$DIR/issue-83150.rs:10:24: 10:33]>, [closure@$DIR/issue-83150.rs:10:24: 10:33]>, [closure@$DIR/issue-83150.rs:10:24: 10:33]>, [closure@$DIR/issue-83150.rs:10:24: 10:33]>, [closure@$DIR/issue-83150.rs:10:24: 10:33]>, [closure@$DIR/issue-83150.rs:10:24: 10:33]>, [closure@$DIR/issue-83150.rs:10:24: 10:33]>, [closure@$DIR/issue-83150.rs:10:24: 10:33]>, [closure@$DIR/issue-83150.rs:10:24: 10:33]>, [closure@$DIR/issue-83150.rs:10:24: 10:33]>, [closure@$DIR/issue-83150.rs:10:24: 10:33]>, [closure@$DIR/issue-83150.rs:10:24: 10:33]>, [closure@$DIR/issue-83150.rs:10:24: 10:33]>, [closure@$DIR/issue-83150.rs:10:24: 10:33]>, [closure@$DIR/issue-83150.rs:10:24: 10:33]>, [closure@$DIR/issue-83150.rs:10:24: 10:33]>, [closure@$DIR/issue-83150.rs:10:24: 10:33]>, [closure@$DIR/issue-83150.rs:10:24: 10:33]>, [closure@$DIR/issue-83150.rs:10:24: 10:33]>, [closure@$DIR/issue-83150.rs:10:24: 10:33]>, [closure@$DIR/issue-83150.rs:10:24: 10:33]>, [closure@$DIR/issue-83150.rs:10:24: 10:33]>, [closure@$DIR/issue-83150.rs:10:24: 10:33]>, [closure@$DIR/issue-83150.rs:10:24: 10:33]>, [closure@$DIR/issue-83150.rs:10:24: 10:33]>, [closure@$DIR/issue-83150.rs:10:24: 10:33]>, [closure@$DIR/issue-83150.rs:10:24: 10:33]>, [closure@$DIR/issue-83150.rs:10:24: 10:33]>, [closure@$DIR/issue-83150.rs:10:24: 10:33]>, [closure@$DIR/issue-83150.rs:10:24: 10:33]>, [closure@$DIR/issue-83150.rs:10:24: 10:33]>, [closure@$DIR/issue-83150.rs:10:24: 10:33]>, [closure@$DIR/issue-83150.rs:10:24: 10:33]>`

error: aborting due to previous error
error: aborting due to previous error; 1 warning emitted

For more information about this error, try `rustc --explain E0275`.

0 comments on commit 21b4a9c

Please sign in to comment.