Skip to content

Commit 1b76a34

Browse files
committed
fix: large_stack_frames FP on compiler generated targets
1 parent 45168a7 commit 1b76a34

File tree

10 files changed

+269
-47
lines changed

10 files changed

+269
-47
lines changed

CHANGELOG.md

Lines changed: 1 addition & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -7081,6 +7081,7 @@ Released 2018-09-13
70817081
[`allow-expect-in-consts`]: https://doc.rust-lang.org/clippy/lint_configuration.html#allow-expect-in-consts
70827082
[`allow-expect-in-tests`]: https://doc.rust-lang.org/clippy/lint_configuration.html#allow-expect-in-tests
70837083
[`allow-indexing-slicing-in-tests`]: https://doc.rust-lang.org/clippy/lint_configuration.html#allow-indexing-slicing-in-tests
7084+
[`allow-large-stack-frames-in-tests`]: https://doc.rust-lang.org/clippy/lint_configuration.html#allow-large-stack-frames-in-tests
70847085
[`allow-mixed-uninlined-format-args`]: https://doc.rust-lang.org/clippy/lint_configuration.html#allow-mixed-uninlined-format-args
70857086
[`allow-one-hash-in-raw-strings`]: https://doc.rust-lang.org/clippy/lint_configuration.html#allow-one-hash-in-raw-strings
70867087
[`allow-panic-in-tests`]: https://doc.rust-lang.org/clippy/lint_configuration.html#allow-panic-in-tests

book/src/lint_configuration.md

Lines changed: 10 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -111,6 +111,16 @@ Whether `indexing_slicing` should be allowed in test functions or `#[cfg(test)]`
111111
* [`indexing_slicing`](https://rust-lang.github.io/rust-clippy/master/index.html#indexing_slicing)
112112

113113

114+
## `allow-large-stack-frames-in-tests`
115+
Whether functions inside `#[cfg(test)]` modules or test functions should be checked.
116+
117+
**Default Value:** `true`
118+
119+
---
120+
**Affected lints:**
121+
* [`large_stack_frames`](https://rust-lang.github.io/rust-clippy/master/index.html#large_stack_frames)
122+
123+
114124
## `allow-mixed-uninlined-format-args`
115125
Whether to allow mixed uninlined format args, e.g. `format!("{} {}", a, foo.bar)`
116126

clippy_config/src/conf.rs

Lines changed: 3 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -373,6 +373,9 @@ define_Conf! {
373373
/// Whether `indexing_slicing` should be allowed in test functions or `#[cfg(test)]`
374374
#[lints(indexing_slicing)]
375375
allow_indexing_slicing_in_tests: bool = false,
376+
/// Whether functions inside `#[cfg(test)]` modules or test functions should be checked.
377+
#[lints(large_stack_frames)]
378+
allow_large_stack_frames_in_tests: bool = true,
376379
/// Whether to allow mixed uninlined format args, e.g. `format!("{} {}", a, foo.bar)`
377380
#[lints(uninlined_format_args)]
378381
allow_mixed_uninlined_format_args: bool = true,

clippy_lints/src/large_stack_frames.rs

Lines changed: 105 additions & 47 deletions
Original file line numberDiff line numberDiff line change
@@ -2,15 +2,16 @@ use std::{fmt, ops};
22

33
use clippy_config::Conf;
44
use clippy_utils::diagnostics::span_lint_and_then;
5-
use clippy_utils::fn_has_unsatisfiable_preds;
6-
use clippy_utils::source::SpanRangeExt;
5+
use clippy_utils::source::{HasSession, SpanRangeExt};
6+
use clippy_utils::{fn_has_unsatisfiable_preds, is_entrypoint_fn, is_in_test};
7+
use rustc_errors::Diag;
78
use rustc_hir::def_id::LocalDefId;
89
use rustc_hir::intravisit::FnKind;
910
use rustc_hir::{Body, FnDecl};
1011
use rustc_lexer::is_ident;
1112
use rustc_lint::{LateContext, LateLintPass};
1213
use rustc_session::impl_lint_pass;
13-
use rustc_span::Span;
14+
use rustc_span::{Span, SyntaxContext};
1415

1516
declare_clippy_lint! {
1617
/// ### What it does
@@ -83,12 +84,14 @@ declare_clippy_lint! {
8384

8485
pub struct LargeStackFrames {
8586
maximum_allowed_size: u64,
87+
allow_large_stack_frames_in_tests: bool,
8688
}
8789

8890
impl LargeStackFrames {
8991
pub fn new(conf: &'static Conf) -> Self {
9092
Self {
9193
maximum_allowed_size: conf.stack_size_threshold,
94+
allow_large_stack_frames_in_tests: conf.allow_large_stack_frames_in_tests,
9295
}
9396
}
9497
}
@@ -152,67 +155,122 @@ impl<'tcx> LateLintPass<'tcx> for LargeStackFrames {
152155
let mir = cx.tcx.optimized_mir(def_id);
153156
let typing_env = mir.typing_env(cx.tcx);
154157

155-
let sizes_of_locals = || {
156-
mir.local_decls.iter().filter_map(|local| {
158+
let sizes_of_locals = mir
159+
.local_decls
160+
.iter()
161+
.filter_map(|local| {
157162
let layout = cx.tcx.layout_of(typing_env.as_query_input(local.ty)).ok()?;
158163
Some((local, layout.size.bytes()))
159164
})
160-
};
165+
.collect::<Vec<_>>();
161166

162-
let frame_size = sizes_of_locals().fold(Space::Used(0), |sum, (_, size)| sum + size);
167+
let frame_size = sizes_of_locals
168+
.iter()
169+
.fold(Space::Used(0), |sum, (_, size)| sum + *size);
163170

164171
let limit = self.maximum_allowed_size;
165172
if frame_size.exceeds_limit(limit) {
166173
// Point at just the function name if possible, because lints that span
167174
// the entire body and don't have to are less legible.
168-
let fn_span = match fn_kind {
169-
FnKind::ItemFn(ident, _, _) | FnKind::Method(ident, _) => ident.span,
170-
FnKind::Closure => entire_fn_span,
175+
let (fn_span, fn_name) = match fn_kind {
176+
FnKind::ItemFn(ident, _, _) => (ident.span, format!("function `{}`", ident.name)),
177+
FnKind::Method(ident, _) => (ident.span, format!("method `{}`", ident.name)),
178+
FnKind::Closure => (entire_fn_span, "closure".to_string()),
171179
};
172180

181+
// Don't lint inside tests if configured to not do so.
182+
if self.allow_large_stack_frames_in_tests && is_in_test(cx.tcx, cx.tcx.local_def_id_to_hir_id(local_def_id))
183+
{
184+
return;
185+
}
186+
187+
let explain_lint = |diag: &mut Diag<'_, ()>, ctxt: SyntaxContext| {
188+
// Point out the largest individual contribution to this size, because
189+
// it is the most likely to be unintentionally large.
190+
if let Some((local, size)) = sizes_of_locals.iter().max_by_key(|&(_, size)| size)
191+
&& let local_span = local.source_info.span
192+
&& local_span.ctxt() == ctxt
193+
{
194+
let size = Space::Used(*size); // pluralizes for us
195+
let ty = local.ty;
196+
197+
// TODO: Is there a cleaner, robust way to ask this question?
198+
// The obvious `LocalDecl::is_user_variable()` panics on "unwrapping cross-crate data",
199+
// and that doesn't get us the true name in scope rather than the span text either.
200+
if let Some(name) = local_span.get_source_text(cx)
201+
&& is_ident(&name)
202+
{
203+
// If the local is an ordinary named variable,
204+
// print its name rather than relying solely on the span.
205+
diag.span_label(
206+
local_span,
207+
format!("`{name}` is the largest part, at {size} for type `{ty}`"),
208+
);
209+
} else {
210+
diag.span_label(
211+
local_span,
212+
format!("this is the largest part, at {size} for type `{ty}`"),
213+
);
214+
}
215+
}
216+
217+
// Explain why we are linting this and not other functions.
218+
diag.note(format!(
219+
"{frame_size} is larger than Clippy's configured `stack-size-threshold` of {limit}"
220+
));
221+
222+
// Explain why the user should care, briefly.
223+
diag.note_once(
224+
"allocating large amounts of stack space can overflow the stack \
225+
and cause the program to abort",
226+
);
227+
};
228+
229+
if fn_span.from_expansion() {
230+
// Don't lint on the main function generated by `--test` target
231+
if cx.sess().is_test_crate() && is_entrypoint_fn(cx, local_def_id.to_def_id()) {
232+
return;
233+
}
234+
235+
let is_from_external_macro = fn_span.in_external_macro(cx.sess().source_map());
236+
span_lint_and_then(
237+
cx,
238+
LARGE_STACK_FRAMES,
239+
fn_span.source_callsite(),
240+
format!(
241+
"{} generated by this macro may allocate a lot of stack space",
242+
if is_from_external_macro {
243+
cx.tcx.def_descr(local_def_id.into())
244+
} else {
245+
fn_name.as_str()
246+
}
247+
),
248+
|diag| {
249+
if is_from_external_macro {
250+
return;
251+
}
252+
253+
diag.span_label(
254+
fn_span,
255+
format!(
256+
"this {} has a stack frame size of {frame_size}",
257+
cx.tcx.def_descr(local_def_id.into())
258+
),
259+
);
260+
261+
explain_lint(diag, fn_span.ctxt());
262+
},
263+
);
264+
return;
265+
}
266+
173267
span_lint_and_then(
174268
cx,
175269
LARGE_STACK_FRAMES,
176270
fn_span,
177271
format!("this function may allocate {frame_size} on the stack"),
178272
|diag| {
179-
// Point out the largest individual contribution to this size, because
180-
// it is the most likely to be unintentionally large.
181-
if let Some((local, size)) = sizes_of_locals().max_by_key(|&(_, size)| size) {
182-
let local_span: Span = local.source_info.span;
183-
let size = Space::Used(size); // pluralizes for us
184-
let ty = local.ty;
185-
186-
// TODO: Is there a cleaner, robust way to ask this question?
187-
// The obvious `LocalDecl::is_user_variable()` panics on "unwrapping cross-crate data",
188-
// and that doesn't get us the true name in scope rather than the span text either.
189-
if let Some(name) = local_span.get_source_text(cx)
190-
&& is_ident(&name)
191-
{
192-
// If the local is an ordinary named variable,
193-
// print its name rather than relying solely on the span.
194-
diag.span_label(
195-
local_span,
196-
format!("`{name}` is the largest part, at {size} for type `{ty}`"),
197-
);
198-
} else {
199-
diag.span_label(
200-
local_span,
201-
format!("this is the largest part, at {size} for type `{ty}`"),
202-
);
203-
}
204-
}
205-
206-
// Explain why we are linting this and not other functions.
207-
diag.note(format!(
208-
"{frame_size} is larger than Clippy's configured `stack-size-threshold` of {limit}"
209-
));
210-
211-
// Explain why the user should care, briefly.
212-
diag.note_once(
213-
"allocating large amounts of stack space can overflow the stack \
214-
and cause the program to abort",
215-
);
273+
explain_lint(diag, SyntaxContext::root());
216274
},
217275
);
218276
}
Lines changed: 1 addition & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1 @@
1+
stack-size-threshold = 0
Lines changed: 42 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,42 @@
1+
//@ignore-target: i686
2+
//@normalize-stderr-test: "\b10000(08|16|32)\b" -> "100$$PTR"
3+
//@normalize-stderr-test: "\b2500(060|120)\b" -> "250$$PTR"
4+
5+
#![warn(clippy::large_stack_frames)]
6+
7+
extern crate serde;
8+
use serde::{Deserialize, Serialize};
9+
10+
struct ArrayDefault<const N: usize>([u8; N]);
11+
12+
macro_rules! mac {
13+
($name:ident) => {
14+
fn foo() {
15+
let $name = 1;
16+
println!("macro_name called");
17+
}
18+
19+
fn bar() {
20+
let $name = ArrayDefault([0; 1000]);
21+
}
22+
};
23+
}
24+
25+
mac!(something);
26+
//~^ large_stack_frames
27+
//~| large_stack_frames
28+
29+
#[derive(Deserialize, Serialize)]
30+
//~^ large_stack_frames
31+
//~| large_stack_frames
32+
//~| large_stack_frames
33+
//~| large_stack_frames
34+
//~| large_stack_frames
35+
//~| large_stack_frames
36+
//~| large_stack_frames
37+
//~| large_stack_frames
38+
struct S {
39+
a: [u128; 31],
40+
}
41+
42+
fn main() {}
Lines changed: 89 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,89 @@
1+
error: function `foo` generated by this macro may allocate a lot of stack space
2+
--> tests/ui-toml/large_stack_frames_for_macros/large_stack_frames.rs:25:1
3+
|
4+
LL | fn foo() {
5+
| --- this function has a stack frame size of 20 bytes
6+
...
7+
LL | mac!(something);
8+
| ^^^^^^^^^^^^^^^
9+
|
10+
= note: 20 bytes is larger than Clippy's configured `stack-size-threshold` of 0
11+
= note: allocating large amounts of stack space can overflow the stack and cause the program to abort
12+
= note: `-D clippy::large-stack-frames` implied by `-D warnings`
13+
= help: to override `-D warnings` add `#[allow(clippy::large_stack_frames)]`
14+
15+
error: function `bar` generated by this macro may allocate a lot of stack space
16+
--> tests/ui-toml/large_stack_frames_for_macros/large_stack_frames.rs:25:1
17+
|
18+
LL | fn bar() {
19+
| --- this function has a stack frame size of 2000 bytes
20+
LL | let $name = ArrayDefault([0; 1000]);
21+
| --------- this is the largest part, at 1000 bytes for type `[u8; 1000]`
22+
...
23+
LL | mac!(something);
24+
| ^^^^^^^^^^^^^^^
25+
|
26+
= note: 2000 bytes is larger than Clippy's configured `stack-size-threshold` of 0
27+
28+
error: method generated by this macro may allocate a lot of stack space
29+
--> tests/ui-toml/large_stack_frames_for_macros/large_stack_frames.rs:29:10
30+
|
31+
LL | #[derive(Deserialize, Serialize)]
32+
| ^^^^^^^^^^^
33+
34+
error: method generated by this macro may allocate a lot of stack space
35+
--> tests/ui-toml/large_stack_frames_for_macros/large_stack_frames.rs:29:10
36+
|
37+
LL | #[derive(Deserialize, Serialize)]
38+
| ^^^^^^^^^^^
39+
|
40+
= note: duplicate diagnostic emitted due to `-Z deduplicate-diagnostics=no`
41+
42+
error: method generated by this macro may allocate a lot of stack space
43+
--> tests/ui-toml/large_stack_frames_for_macros/large_stack_frames.rs:29:10
44+
|
45+
LL | #[derive(Deserialize, Serialize)]
46+
| ^^^^^^^^^^^
47+
|
48+
= note: duplicate diagnostic emitted due to `-Z deduplicate-diagnostics=no`
49+
50+
error: method generated by this macro may allocate a lot of stack space
51+
--> tests/ui-toml/large_stack_frames_for_macros/large_stack_frames.rs:29:10
52+
|
53+
LL | #[derive(Deserialize, Serialize)]
54+
| ^^^^^^^^^^^
55+
|
56+
= note: duplicate diagnostic emitted due to `-Z deduplicate-diagnostics=no`
57+
58+
error: method generated by this macro may allocate a lot of stack space
59+
--> tests/ui-toml/large_stack_frames_for_macros/large_stack_frames.rs:29:10
60+
|
61+
LL | #[derive(Deserialize, Serialize)]
62+
| ^^^^^^^^^^^
63+
|
64+
= note: duplicate diagnostic emitted due to `-Z deduplicate-diagnostics=no`
65+
66+
error: method generated by this macro may allocate a lot of stack space
67+
--> tests/ui-toml/large_stack_frames_for_macros/large_stack_frames.rs:29:10
68+
|
69+
LL | #[derive(Deserialize, Serialize)]
70+
| ^^^^^^^^^^^
71+
|
72+
= note: duplicate diagnostic emitted due to `-Z deduplicate-diagnostics=no`
73+
74+
error: method generated by this macro may allocate a lot of stack space
75+
--> tests/ui-toml/large_stack_frames_for_macros/large_stack_frames.rs:29:10
76+
|
77+
LL | #[derive(Deserialize, Serialize)]
78+
| ^^^^^^^^^^^
79+
|
80+
= note: duplicate diagnostic emitted due to `-Z deduplicate-diagnostics=no`
81+
82+
error: method generated by this macro may allocate a lot of stack space
83+
--> tests/ui-toml/large_stack_frames_for_macros/large_stack_frames.rs:29:23
84+
|
85+
LL | #[derive(Deserialize, Serialize)]
86+
| ^^^^^^^^^
87+
88+
error: aborting due to 10 previous errors
89+
Lines changed: 2 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,2 @@
1+
stack-size-threshold = 0
2+
allow-large-stack-frames-in-tests = false
Lines changed: 13 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,13 @@
1+
// This test checks if `clippy::large_stack_frames` is working correctly when encountering functions
2+
// generated by special compiling targets like `--test`.
3+
//@compile-flags: --test
4+
//@check-pass
5+
6+
#![warn(clippy::large_stack_frames)]
7+
8+
#[cfg(test)]
9+
#[expect(clippy::large_stack_frames)]
10+
mod test {
11+
#[test]
12+
fn main_test() {}
13+
}

0 commit comments

Comments
 (0)