-
Notifications
You must be signed in to change notification settings - Fork 1.7k
new lint: large_stack_frames
#10827
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.
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
Merged
Merged
new lint: large_stack_frames
#10827
Changes from all commits
Commits
Show all changes
8 commits
Select commit
Hold shift + click to select a range
7312a93
new lint: `large_stack_frames`
y21 760f91f
`cargo dev update_lints`
y21 52c6ef7
fmt
y21 6b232fd
use span_lint_and_note
y21 7aa4bab
rename "drawbacks" to "known issues"
y21 de7d43b
make lint description easier to read, prevent ICE
y21 c8c7a3c
fix rebase weirdness
y21 6ad7c6f
update lint_configuration.md
y21 File filter
Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
There are no files selected for viewing
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Original file line number | Diff line number | Diff line change |
---|---|---|
@@ -0,0 +1,162 @@ | ||
use std::ops::AddAssign; | ||
|
||
use clippy_utils::diagnostics::span_lint_and_note; | ||
use clippy_utils::fn_has_unsatisfiable_preds; | ||
use rustc_hir::def_id::LocalDefId; | ||
use rustc_hir::intravisit::FnKind; | ||
use rustc_hir::Body; | ||
use rustc_hir::FnDecl; | ||
use rustc_lint::{LateContext, LateLintPass}; | ||
use rustc_session::declare_tool_lint; | ||
use rustc_session::impl_lint_pass; | ||
use rustc_span::Span; | ||
|
||
declare_clippy_lint! { | ||
/// ### What it does | ||
/// Checks for functions that use a lot of stack space. | ||
/// | ||
/// This often happens when constructing a large type, such as an array with a lot of elements, | ||
/// or constructing *many* smaller-but-still-large structs, or copying around a lot of large types. | ||
/// | ||
/// This lint is a more general version of [`large_stack_arrays`](https://rust-lang.github.io/rust-clippy/master/#large_stack_arrays) | ||
/// that is intended to look at functions as a whole instead of only individual array expressions inside of a function. | ||
/// | ||
/// ### Why is this bad? | ||
/// The stack region of memory is very limited in size (usually *much* smaller than the heap) and attempting to | ||
/// use too much will result in a stack overflow and crash the program. | ||
/// To avoid this, you should consider allocating large types on the heap instead (e.g. by boxing them). | ||
/// | ||
/// Keep in mind that the code path to construction of large types does not even need to be reachable; | ||
/// it purely needs to *exist* inside of the function to contribute to the stack size. | ||
/// For example, this causes a stack overflow even though the branch is unreachable: | ||
/// ```rust,ignore | ||
/// fn main() { | ||
/// if false { | ||
/// let x = [0u8; 10000000]; // 10 MB stack array | ||
/// black_box(&x); | ||
/// } | ||
/// } | ||
/// ``` | ||
/// | ||
/// ### Known issues | ||
/// False positives. The stack size that clippy sees is an estimated value and can be vastly different | ||
/// from the actual stack usage after optimizations passes have run (especially true in release mode). | ||
/// Modern compilers are very smart and are able to optimize away a lot of unnecessary stack allocations. | ||
/// In debug mode however, it is usually more accurate. | ||
/// | ||
/// This lint works by summing up the size of all variables that the user typed, variables that were | ||
/// implicitly introduced by the compiler for temporaries, function arguments and the return value, | ||
/// and comparing them against a (configurable, but high-by-default). | ||
/// | ||
/// ### Example | ||
/// This function creates four 500 KB arrays on the stack. Quite big but just small enough to not trigger `large_stack_arrays`. | ||
/// However, looking at the function as a whole, it's clear that this uses a lot of stack space. | ||
/// ```rust | ||
/// struct QuiteLargeType([u8; 500_000]); | ||
/// fn foo() { | ||
/// // ... some function that uses a lot of stack space ... | ||
/// let _x1 = QuiteLargeType([0; 500_000]); | ||
/// let _x2 = QuiteLargeType([0; 500_000]); | ||
/// let _x3 = QuiteLargeType([0; 500_000]); | ||
/// let _x4 = QuiteLargeType([0; 500_000]); | ||
/// } | ||
/// ``` | ||
/// | ||
/// Instead of doing this, allocate the arrays on the heap. | ||
/// This currently requires going through a `Vec` first and then converting it to a `Box`: | ||
/// ```rust | ||
/// struct NotSoLargeType(Box<[u8]>); | ||
/// | ||
/// fn foo() { | ||
/// let _x1 = NotSoLargeType(vec![0; 500_000].into_boxed_slice()); | ||
/// // ^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^ Now heap allocated. | ||
/// // The size of `NotSoLargeType` is 16 bytes. | ||
/// // ... | ||
/// } | ||
/// ``` | ||
#[clippy::version = "1.71.0"] | ||
pub LARGE_STACK_FRAMES, | ||
nursery, | ||
"checks for functions that allocate a lot of stack space" | ||
} | ||
|
||
pub struct LargeStackFrames { | ||
maximum_allowed_size: u64, | ||
} | ||
|
||
impl LargeStackFrames { | ||
#[must_use] | ||
pub fn new(size: u64) -> Self { | ||
Self { | ||
maximum_allowed_size: size, | ||
} | ||
} | ||
} | ||
|
||
impl_lint_pass!(LargeStackFrames => [LARGE_STACK_FRAMES]); | ||
|
||
#[derive(Copy, Clone)] | ||
enum Space { | ||
Used(u64), | ||
Overflow, | ||
} | ||
|
||
impl Space { | ||
pub fn exceeds_limit(self, limit: u64) -> bool { | ||
match self { | ||
Self::Used(used) => used > limit, | ||
Self::Overflow => true, | ||
} | ||
} | ||
} | ||
|
||
impl AddAssign<u64> for Space { | ||
fn add_assign(&mut self, rhs: u64) { | ||
if let Self::Used(lhs) = self { | ||
match lhs.checked_add(rhs) { | ||
Some(sum) => *self = Self::Used(sum), | ||
None => *self = Self::Overflow, | ||
} | ||
} | ||
} | ||
} | ||
|
||
impl<'tcx> LateLintPass<'tcx> for LargeStackFrames { | ||
fn check_fn( | ||
&mut self, | ||
cx: &LateContext<'tcx>, | ||
_: FnKind<'tcx>, | ||
_: &'tcx FnDecl<'tcx>, | ||
_: &'tcx Body<'tcx>, | ||
span: Span, | ||
local_def_id: LocalDefId, | ||
) { | ||
let def_id = local_def_id.to_def_id(); | ||
// Building MIR for `fn`s with unsatisfiable preds results in ICE. | ||
if fn_has_unsatisfiable_preds(cx, def_id) { | ||
return; | ||
} | ||
y21 marked this conversation as resolved.
Show resolved
Hide resolved
|
||
|
||
let mir = cx.tcx.optimized_mir(def_id); | ||
let param_env = cx.tcx.param_env(def_id); | ||
|
||
let mut frame_size = Space::Used(0); | ||
|
||
for local in &mir.local_decls { | ||
if let Ok(layout) = cx.tcx.layout_of(param_env.and(local.ty)) { | ||
frame_size += layout.size.bytes(); | ||
} | ||
} | ||
|
||
if frame_size.exceeds_limit(self.maximum_allowed_size) { | ||
span_lint_and_note( | ||
cx, | ||
LARGE_STACK_FRAMES, | ||
span, | ||
"this function allocates a large amount of stack space", | ||
None, | ||
"allocating large amounts of stack space can overflow the stack", | ||
); | ||
} | ||
} | ||
} |
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Original file line number | Diff line number | Diff line change |
---|---|---|
@@ -0,0 +1,44 @@ | ||
#![allow(unused, incomplete_features)] | ||
#![warn(clippy::large_stack_frames)] | ||
#![feature(unsized_locals)] | ||
|
||
use std::hint::black_box; | ||
|
||
fn generic<T: Default>() { | ||
let x = T::default(); | ||
black_box(&x); | ||
} | ||
|
||
fn unsized_local() { | ||
let x: dyn std::fmt::Display = *(Box::new(1) as Box<dyn std::fmt::Display>); | ||
black_box(&x); | ||
} | ||
|
||
struct ArrayDefault<const N: usize>([u8; N]); | ||
|
||
impl<const N: usize> Default for ArrayDefault<N> { | ||
fn default() -> Self { | ||
Self([0; N]) | ||
} | ||
} | ||
|
||
fn many_small_arrays() { | ||
let x = [0u8; 500_000]; | ||
let x2 = [0u8; 500_000]; | ||
let x3 = [0u8; 500_000]; | ||
let x4 = [0u8; 500_000]; | ||
let x5 = [0u8; 500_000]; | ||
black_box((&x, &x2, &x3, &x4, &x5)); | ||
} | ||
|
||
fn large_return_value() -> ArrayDefault<1_000_000> { | ||
Default::default() | ||
} | ||
|
||
fn large_fn_arg(x: ArrayDefault<1_000_000>) { | ||
black_box(&x); | ||
} | ||
|
||
fn main() { | ||
generic::<ArrayDefault<1_000_000>>(); | ||
} |
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Original file line number | Diff line number | Diff line change |
---|---|---|
@@ -0,0 +1,37 @@ | ||
error: this function allocates a large amount of stack space | ||
--> $DIR/large_stack_frames.rs:25:1 | ||
| | ||
LL | / fn many_small_arrays() { | ||
LL | | let x = [0u8; 500_000]; | ||
LL | | let x2 = [0u8; 500_000]; | ||
LL | | let x3 = [0u8; 500_000]; | ||
... | | ||
LL | | black_box((&x, &x2, &x3, &x4, &x5)); | ||
LL | | } | ||
| |_^ | ||
| | ||
= note: allocating large amounts of stack space can overflow the stack | ||
= note: `-D clippy::large-stack-frames` implied by `-D warnings` | ||
|
||
error: this function allocates a large amount of stack space | ||
--> $DIR/large_stack_frames.rs:34:1 | ||
| | ||
LL | / fn large_return_value() -> ArrayDefault<1_000_000> { | ||
LL | | Default::default() | ||
LL | | } | ||
| |_^ | ||
| | ||
= note: allocating large amounts of stack space can overflow the stack | ||
|
||
error: this function allocates a large amount of stack space | ||
--> $DIR/large_stack_frames.rs:38:1 | ||
| | ||
LL | / fn large_fn_arg(x: ArrayDefault<1_000_000>) { | ||
LL | | black_box(&x); | ||
LL | | } | ||
| |_^ | ||
| | ||
= note: allocating large amounts of stack space can overflow the stack | ||
|
||
error: aborting due to 3 previous errors | ||
|
Add this suggestion to a batch that can be applied as a single commit.
This suggestion is invalid because no changes were made to the code.
Suggestions cannot be applied while the pull request is closed.
Suggestions cannot be applied while viewing a subset of changes.
Only one suggestion per line can be applied in a batch.
Add this suggestion to a batch that can be applied as a single commit.
Applying suggestions on deleted lines is not supported.
You must change the existing code in this line in order to create a valid suggestion.
Outdated suggestions cannot be applied.
This suggestion has been applied or marked resolved.
Suggestions cannot be applied from pending reviews.
Suggestions cannot be applied on multi-line comments.
Suggestions cannot be applied while the pull request is queued to merge.
Suggestion cannot be applied right now. Please check back later.
Uh oh!
There was an error while loading. Please reload this page.