Fix underflow in manual_split_once lint

[pr2502]

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

The mostly minimized repro is:

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

[rust-highfive]

Thanks for the pull request, and welcome! The Rust team is excited to review your changes, and you should hear from @giraffate (or someone else) soon.

Please see the contribution instructions for more information.

[BlackHoleFox]

(I'm the friend)

This was introduced when the lint was added and subsequently started affecting nightlies with 09-10-2021 when it was merged to rustc.

[Jarcho]

A better fix would be to change the first if to if adjust.len < 2 instead of if adjust.is_empty().

[giraffate]

Thanks!

Can you add a test?

[pr2502]

Definitely, I wasn't sure where to put it though. Is it supposed to be a full UI test of the lint (I couldn't find one for this lint), or is there already one and I should just add the failing code from the description, or is there some place where you'd put regression tests for bugs like this?

[giraffate]

You can write code from description and put a test here: https://github.com/rust-lang/rust-clippy/blob/master/tests/ui/manual_split_once.rs
FYI this document will help you to write tests: https://github.com/rust-lang/rust-clippy/blob/master/doc/adding_lints.md#testing

The right name of the lint is manual_split_once, so I changed title and changelog. If you have any other questions, feel free to ask me.

[flip1995]

Since this is a ICE fix, we usually put those in tests/ui/crashes where the filename is usually the issue number. AFAICT this doesn't have an issue, so you can use the PR number.

[pr2502]

Sorry for the delay, I've added the test to tests/ui/crashes/ice-8250.rs as @flip1995 suggested.

[giraffate]

@bors r+

It looks good, thanks!

[bors]

📌 Commit 23fd95a has been approved by giraffate

[bors]

⌛ Testing commit 23fd95a with merge 8d5d9e0...

[bors]

☀️ Test successful - checks-action_dev_test, checks-action_remark_test, checks-action_test
Approved by: giraffate
Pushing 8d5d9e0 to master...