Skip to content

Commit

Permalink
Auto merge of #8250 - pr2502:fix_repeat_underflow, r=giraffate
Browse files Browse the repository at this point in the history
Fix underflow in `manual_split_once` lint

Hi, a friend found clippy started crashing on a suspiciously large allocation of `u64::MAX` memory on their code.

The mostly minimized repro is:
```rust
fn _f01(title: &str) -> Option<()> {
    let _ = title[1..].splitn(2, '[').next()?;
    Some(())
}
```

The underflow happens in this case on line 57 of the patch but I've changed the other substraction to saturating as well since it could potentially cause the same issue.

I'm not sure where to put a regression test, or if it's even worth for such a thing.

Aside, has it been considered before to build clippy with overflow checks enabled?

changelog: fix ICE of underflow in `manual_split_once` lint
  • Loading branch information
bors committed Jan 28, 2022
2 parents fb94992 + 23fd95a commit 8d5d9e0
Show file tree
Hide file tree
Showing 3 changed files with 27 additions and 3 deletions.
6 changes: 3 additions & 3 deletions clippy_lints/src/methods/str_splitn.rs
Original file line number Diff line number Diff line change
Expand Up @@ -45,16 +45,16 @@ pub(super) fn check_manual_split_once(
IterUsageKind::Next | IterUsageKind::Second => {
let self_deref = {
let adjust = cx.typeck_results().expr_adjustments(self_arg);
if adjust.is_empty() {
if adjust.len() < 2 {
String::new()
} else if cx.typeck_results().expr_ty(self_arg).is_box()
|| adjust
.iter()
.any(|a| matches!(a.kind, Adjust::Deref(Some(_))) || a.target.is_box())
{
format!("&{}", "*".repeat(adjust.len() - 1))
format!("&{}", "*".repeat(adjust.len().saturating_sub(1)))
} else {
"*".repeat(adjust.len() - 2)
"*".repeat(adjust.len().saturating_sub(2))
}
};
if matches!(usage.kind, IterUsageKind::Next) {
Expand Down
6 changes: 6 additions & 0 deletions tests/ui/crashes/ice-8250.rs
Original file line number Diff line number Diff line change
@@ -0,0 +1,6 @@
fn _f(s: &str) -> Option<()> {
let _ = s[1..].splitn(2, '.').next()?;
Some(())
}

fn main() {}
18 changes: 18 additions & 0 deletions tests/ui/crashes/ice-8250.stderr
Original file line number Diff line number Diff line change
@@ -0,0 +1,18 @@
error: manual implementation of `split_once`
--> $DIR/ice-8250.rs:2:13
|
LL | let _ = s[1..].splitn(2, '.').next()?;
| ^^^^^^^^^^^^^^^^^^^^^^^^^^^^^ help: try this: `s[1..].split_once('.').map_or(s[1..], |x| x.0)`
|
= note: `-D clippy::manual-split-once` implied by `-D warnings`

error: unnecessary use of `splitn`
--> $DIR/ice-8250.rs:2:13
|
LL | let _ = s[1..].splitn(2, '.').next()?;
| ^^^^^^^^^^^^^^^^^^^^^ help: try this: `s[1..].split('.')`
|
= note: `-D clippy::needless-splitn` implied by `-D warnings`

error: aborting due to 2 previous errors

0 comments on commit 8d5d9e0

Please sign in to comment.