rust-lang / rust-clippy

A bunch of lints to catch common mistakes and improve your Rust code. Book: https://doc.rust-lang.org/clippy/
https://rust-lang.github.io/rust-clippy/
Other
11.27k stars 1.52k forks source link

huge memory consumption on testcase #4630

Open matthiaskrgr opened 4 years ago

matthiaskrgr commented 4 years ago

cc https://github.com/rust-lang/rust-clippy/issues/3142 This was found by running clippy on glacier https://github.com/rust-lang/glacier ./fixed/23600.rs

static V1: [u8; 0xffff_ffff] = [0u8; 0xffff_ffff];
fn main() { V1[0]; }

seems to want to consume all of the systems run when checked with clippy.

clippy 0.0.212 (648e5b9 2019-10-01)

flip1995 commented 4 years ago

Clippy just uses a few MBs more than just running rustc main.rs on this file. Same behavior with cargo check. I don't think that this is a bug, since this allocates a static array of 4GB. Since statics are written in the BSS segment of a program this is quiet expected.

The resulting size of the binary should also be about 4GB.


rustc main.rs also got killed on my OS.

llogiq commented 4 years ago

We could perhaps solve this by extending clippy_lints/src/const.rs so that the static perhaps doesn't even get instantiated.

flip1995 commented 4 years ago

cargo check also allocates these statics, so the only way to prevent an allocation like this is to stop the compilation / check before rustc allocates the static (I don't know at which step this happens). Otherwise we would have to include something in Clippy, that changes basic behavior of rustc (allocation of statics), which I don't think we should do.

That being said, maybe we can change the behavior of cargo-check to not allocate statics. But then again, I can imagine, that MIR relies on these allocations, for optimizations, const propagation or similar things. This would mean, that we need to change the behavior of rustc.

All in all, I don't think Clippy is the right place to take care of this.

flip1995 commented 4 years ago

Also see the two comments by @pnkfelix in the rustc issue:

  1. https://github.com/rust-lang/rust/issues/23600#issuecomment-498288345
  2. https://github.com/rust-lang/rust/issues/23600#issuecomment-498293339
llogiq commented 4 years ago

I agree that we're not exactly going to fix the issue. We could still deny-lint it before MIR gets to see it.