Skip to content

Commit c25cda9

Browse files
lukaslueglukaslueg
committed
Initial impl repr_packed_without_abi
Fixes #13375
1 parent 2536745 commit c25cda9

12 files changed

+188
-26
lines changed

‎CHANGELOG.md

+1
Original file line numberDiff line numberDiff line change
@@ -5878,6 +5878,7 @@ Released 2018-09-13
58785878
[`repeat_once`]: https://rust-lang.github.io/rust-clippy/master/index.html#repeat_once
58795879
[`repeat_vec_with_capacity`]: https://rust-lang.github.io/rust-clippy/master/index.html#repeat_vec_with_capacity
58805880
[`replace_consts`]: https://rust-lang.github.io/rust-clippy/master/index.html#replace_consts
5881+
[`repr_packed_without_abi`]: https://rust-lang.github.io/rust-clippy/master/index.html#repr_packed_without_abi
58815882
[`reserve_after_initialization`]: https://rust-lang.github.io/rust-clippy/master/index.html#reserve_after_initialization
58825883
[`rest_pat_in_fully_bound_structs`]: https://rust-lang.github.io/rust-clippy/master/index.html#rest_pat_in_fully_bound_structs
58835884
[`result_expect_used`]: https://rust-lang.github.io/rust-clippy/master/index.html#result_expect_used

‎clippy_lints/src/attrs/mod.rs

+41
Original file line numberDiff line numberDiff line change
@@ -7,6 +7,7 @@ mod duplicated_attributes;
77
mod inline_always;
88
mod mixed_attributes_style;
99
mod non_minimal_cfg;
10+
mod repr_attributes;
1011
mod should_panic_without_expect;
1112
mod unnecessary_clippy_cfg;
1213
mod useless_attribute;
@@ -272,6 +273,44 @@ declare_clippy_lint! {
272273
"ensures that all `should_panic` attributes specify its expected panic message"
273274
}
274275

276+
declare_clippy_lint! {
277+
/// ### What it does
278+
/// Checks for items with `#[repr(packed)]`-attribute without ABI qualification
279+
///
280+
/// ### Why is this bad?
281+
/// Without qualification, `repr(packed)` implies `repr(Rust)`. The Rust-ABI is inherently unstable.
282+
/// While this is fine as long as the type is accessed correctly within Rust-code, most uses
283+
/// of `#[repr(packed)]` involve FFI and/or data structures specified by network-protocols or
284+
/// other external specifications. In such situations, the unstable Rust-ABI implied in
285+
/// `#[repr(packed)]` may lead to future bugs should the Rust-ABI change.
286+
///
287+
/// In case you are relying on a well defined and stable memory layout, qualify the type's
288+
/// representation using the `C`-ABI. Otherwise, if the type in question is only ever
289+
/// accessed from Rust-code according to Rust's rules, use the `Rust`-ABI explicitly.
290+
///
291+
/// ### Example
292+
/// ```no_run
293+
/// #[repr(packed)]
294+
/// struct NetworkPacketHeader {
295+
/// header_length: u8,
296+
/// header_version: u16
297+
/// }
298+
/// ```
299+
///
300+
/// Use instead:
301+
/// ```no_run
302+
/// #[repr(C, packed)]
303+
/// struct NetworkPacketHeader {
304+
/// header_length: u8,
305+
/// header_version: u16
306+
/// }
307+
/// ```
308+
#[clippy::version = "1.83.0"]
309+
pub REPR_PACKED_WITHOUT_ABI,
310+
suspicious,
311+
"ensures that `repr(packed)` always comes with a qualified ABI"
312+
}
313+
275314
declare_clippy_lint! {
276315
/// ### What it does
277316
/// Checks for `any` and `all` combinators in `cfg` with only one condition.
@@ -421,6 +460,7 @@ impl_lint_pass!(Attributes => [
421460
USELESS_ATTRIBUTE,
422461
BLANKET_CLIPPY_RESTRICTION_LINTS,
423462
SHOULD_PANIC_WITHOUT_EXPECT,
463+
REPR_PACKED_WITHOUT_ABI,
424464
MIXED_ATTRIBUTES_STYLE,
425465
DUPLICATED_ATTRIBUTES,
426466
]);
@@ -481,6 +521,7 @@ impl<'tcx> LateLintPass<'tcx> for Attributes {
481521
}
482522
mixed_attributes_style::check(cx, item.span, attrs);
483523
duplicated_attributes::check(cx, attrs);
524+
repr_attributes::check(cx, item.span, attrs);
484525
}
485526

486527
fn check_impl_item(&mut self, cx: &LateContext<'tcx>, item: &'tcx ImplItem<'_>) {

‎clippy_lints/src/attrs/repr_attributes.rs

+40
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,40 @@
1+
use rustc_ast::Attribute;
2+
use rustc_lint::LateContext;
3+
use rustc_span::{sym, Span};
4+
5+
use clippy_utils::diagnostics::span_lint_and_then;
6+
7+
use super::REPR_PACKED_WITHOUT_ABI;
8+
9+
pub(super) fn check(cx: &LateContext<'_>, item_span: Span, attrs: &[Attribute]) {
10+
check_packed(cx, item_span, attrs);
11+
}
12+
13+
fn check_packed(cx: &LateContext<'_>, item_span: Span, attrs: &[Attribute]) {
14+
if let Some(items) = attrs.iter().find_map(|attr| {
15+
if attr.ident().is_some_and(|ident| matches!(ident.name, sym::repr)) {
16+
attr.meta_item_list()
17+
} else {
18+
None
19+
}
20+
}) && let Some(packed) = items
21+
.iter()
22+
.find(|item| item.ident().is_some_and(|ident| matches!(ident.name, sym::packed)))
23+
&& !items.iter().any(|item| {
24+
item.ident()
25+
.is_some_and(|ident| matches!(ident.name, sym::C | sym::Rust))
26+
})
27+
{
28+
span_lint_and_then(
29+
cx,
30+
REPR_PACKED_WITHOUT_ABI,
31+
item_span,
32+
"item uses `packed` representation without ABI-qualification",
33+
|diag| {
34+
diag.warn("unqualified `#[repr(packed)]` defaults to `#[repr(Rust, packed)]`, which has no stable ABI")
35+
.help("qualify the desired ABI explicity via `#[repr(C, packed)]` or `#[repr(Rust, packed)]`")
36+
.span_label(packed.span(), "`packed` representation set here");
37+
},
38+
);
39+
}
40+
}

‎clippy_lints/src/declared_lints.rs

+1
Original file line numberDiff line numberDiff line change
@@ -52,6 +52,7 @@ pub static LINTS: &[&crate::LintInfo] = &[
5252
crate::attrs::INLINE_ALWAYS_INFO,
5353
crate::attrs::MIXED_ATTRIBUTES_STYLE_INFO,
5454
crate::attrs::NON_MINIMAL_CFG_INFO,
55+
crate::attrs::REPR_PACKED_WITHOUT_ABI_INFO,
5556
crate::attrs::SHOULD_PANIC_WITHOUT_EXPECT_INFO,
5657
crate::attrs::UNNECESSARY_CLIPPY_CFG_INFO,
5758
crate::attrs::USELESS_ATTRIBUTE_INFO,

‎tests/ui/default_union_representation.rs

+1
Original file line numberDiff line numberDiff line change
@@ -1,5 +1,6 @@
11
#![feature(transparent_unions)]
22
#![warn(clippy::default_union_representation)]
3+
#![allow(clippy::repr_packed_without_abi)]
34

45
union NoAttribute {
56
//~^ ERROR: this union has the default representation

‎tests/ui/default_union_representation.stderr

+4-4
Original file line numberDiff line numberDiff line change
@@ -1,5 +1,5 @@
11
error: this union has the default representation
2-
--> tests/ui/default_union_representation.rs:4:1
2+
--> tests/ui/default_union_representation.rs:5:1
33
|
44
LL | / union NoAttribute {
55
LL | |
@@ -13,7 +13,7 @@ LL | | }
1313
= help: to override `-D warnings` add `#[allow(clippy::default_union_representation)]`
1414

1515
error: this union has the default representation
16-
--> tests/ui/default_union_representation.rs:17:1
16+
--> tests/ui/default_union_representation.rs:18:1
1717
|
1818
LL | / union ReprPacked {
1919
LL | |
@@ -25,7 +25,7 @@ LL | | }
2525
= help: consider annotating `ReprPacked` with `#[repr(C)]` to explicitly specify memory layout
2626

2727
error: this union has the default representation
28-
--> tests/ui/default_union_representation.rs:36:1
28+
--> tests/ui/default_union_representation.rs:37:1
2929
|
3030
LL | / union ReprAlign {
3131
LL | |
@@ -37,7 +37,7 @@ LL | | }
3737
= help: consider annotating `ReprAlign` with `#[repr(C)]` to explicitly specify memory layout
3838

3939
error: this union has the default representation
40-
--> tests/ui/default_union_representation.rs:57:1
40+
--> tests/ui/default_union_representation.rs:58:1
4141
|
4242
LL | / union ZSTAndTwoFields {
4343
LL | |

‎tests/ui/derive.rs

+6-1
Original file line numberDiff line numberDiff line change
@@ -1,4 +1,9 @@
1-
#![allow(clippy::non_canonical_clone_impl, clippy::non_canonical_partial_ord_impl, dead_code)]
1+
#![allow(
2+
clippy::non_canonical_clone_impl,
3+
clippy::non_canonical_partial_ord_impl,
4+
clippy::repr_packed_without_abi,
5+
dead_code
6+
)]
27
#![warn(clippy::expl_impl_clone_on_copy)]
38

49
#[derive(Copy)]

‎tests/ui/derive.stderr

+10-10
Original file line numberDiff line numberDiff line change
@@ -1,5 +1,5 @@
11
error: you are implementing `Clone` explicitly on a `Copy` type
2-
--> tests/ui/derive.rs:7:1
2+
--> tests/ui/derive.rs:12:1
33
|
44
LL | / impl Clone for Qux {
55
LL | |
@@ -10,7 +10,7 @@ LL | | }
1010
| |_^
1111
|
1212
note: consider deriving `Clone` or removing `Copy`
13-
--> tests/ui/derive.rs:7:1
13+
--> tests/ui/derive.rs:12:1
1414
|
1515
LL | / impl Clone for Qux {
1616
LL | |
@@ -23,7 +23,7 @@ LL | | }
2323
= help: to override `-D warnings` add `#[allow(clippy::expl_impl_clone_on_copy)]`
2424

2525
error: you are implementing `Clone` explicitly on a `Copy` type
26-
--> tests/ui/derive.rs:32:1
26+
--> tests/ui/derive.rs:37:1
2727
|
2828
LL | / impl<'a> Clone for Lt<'a> {
2929
LL | |
@@ -34,7 +34,7 @@ LL | | }
3434
| |_^
3535
|
3636
note: consider deriving `Clone` or removing `Copy`
37-
--> tests/ui/derive.rs:32:1
37+
--> tests/ui/derive.rs:37:1
3838
|
3939
LL | / impl<'a> Clone for Lt<'a> {
4040
LL | |
@@ -45,7 +45,7 @@ LL | | }
4545
| |_^
4646

4747
error: you are implementing `Clone` explicitly on a `Copy` type
48-
--> tests/ui/derive.rs:44:1
48+
--> tests/ui/derive.rs:49:1
4949
|
5050
LL | / impl Clone for BigArray {
5151
LL | |
@@ -56,7 +56,7 @@ LL | | }
5656
| |_^
5757
|
5858
note: consider deriving `Clone` or removing `Copy`
59-
--> tests/ui/derive.rs:44:1
59+
--> tests/ui/derive.rs:49:1
6060
|
6161
LL | / impl Clone for BigArray {
6262
LL | |
@@ -67,7 +67,7 @@ LL | | }
6767
| |_^
6868

6969
error: you are implementing `Clone` explicitly on a `Copy` type
70-
--> tests/ui/derive.rs:56:1
70+
--> tests/ui/derive.rs:61:1
7171
|
7272
LL | / impl Clone for FnPtr {
7373
LL | |
@@ -78,7 +78,7 @@ LL | | }
7878
| |_^
7979
|
8080
note: consider deriving `Clone` or removing `Copy`
81-
--> tests/ui/derive.rs:56:1
81+
--> tests/ui/derive.rs:61:1
8282
|
8383
LL | / impl Clone for FnPtr {
8484
LL | |
@@ -89,7 +89,7 @@ LL | | }
8989
| |_^
9090

9191
error: you are implementing `Clone` explicitly on a `Copy` type
92-
--> tests/ui/derive.rs:77:1
92+
--> tests/ui/derive.rs:82:1
9393
|
9494
LL | / impl<T: Clone> Clone for Generic2<T> {
9595
LL | |
@@ -100,7 +100,7 @@ LL | | }
100100
| |_^
101101
|
102102
note: consider deriving `Clone` or removing `Copy`
103-
--> tests/ui/derive.rs:77:1
103+
--> tests/ui/derive.rs:82:1
104104
|
105105
LL | / impl<T: Clone> Clone for Generic2<T> {
106106
LL | |

‎tests/ui/repr_packed_without_abi.rs

+37
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,37 @@
1+
#![deny(clippy::repr_packed_without_abi)]
2+
3+
#[repr(packed)]
4+
struct NetworkPacketHeader {
5+
header_length: u8,
6+
header_version: u16,
7+
}
8+
9+
#[repr(packed)]
10+
union Foo {
11+
a: u8,
12+
b: u16,
13+
}
14+
15+
#[repr(C, packed)]
16+
struct NoLintCNetworkPacketHeader {
17+
header_length: u8,
18+
header_version: u16,
19+
}
20+
21+
#[repr(Rust, packed)]
22+
struct NoLintRustNetworkPacketHeader {
23+
header_length: u8,
24+
header_version: u16,
25+
}
26+
27+
#[repr(packed, C)]
28+
union NotLintCFoo {
29+
a: u8,
30+
b: u16,
31+
}
32+
33+
#[repr(packed, Rust)]
34+
union NotLintRustFoo {
35+
a: u8,
36+
b: u16,
37+
}

‎tests/ui/repr_packed_without_abi.stderr

+35
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,35 @@
1+
error: item uses `packed` representation without ABI-qualification
2+
--> tests/ui/repr_packed_without_abi.rs:4:1
3+
|
4+
LL | #[repr(packed)]
5+
| ------ `packed` representation set here
6+
LL | / struct NetworkPacketHeader {
7+
LL | | header_length: u8,
8+
LL | | header_version: u16,
9+
LL | | }
10+
| |_^
11+
|
12+
= warning: unqualified `#[repr(packed)]` defaults to `#[repr(Rust, packed)]`, which has no stable ABI
13+
= help: qualify the desired ABI explicity via `#[repr(C, packed)]` or `#[repr(Rust, packed)]`
14+
note: the lint level is defined here
15+
--> tests/ui/repr_packed_without_abi.rs:1:9
16+
|
17+
LL | #![deny(clippy::repr_packed_without_abi)]
18+
| ^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^
19+
20+
error: item uses `packed` representation without ABI-qualification
21+
--> tests/ui/repr_packed_without_abi.rs:10:1
22+
|
23+
LL | #[repr(packed)]
24+
| ------ `packed` representation set here
25+
LL | / union Foo {
26+
LL | | a: u8,
27+
LL | | b: u16,
28+
LL | | }
29+
| |_^
30+
|
31+
= warning: unqualified `#[repr(packed)]` defaults to `#[repr(Rust, packed)]`, which has no stable ABI
32+
= help: qualify the desired ABI explicity via `#[repr(C, packed)]` or `#[repr(Rust, packed)]`
33+
34+
error: aborting due to 2 previous errors
35+

‎tests/ui/trailing_empty_array.rs

+1
Original file line numberDiff line numberDiff line change
@@ -1,4 +1,5 @@
11
#![warn(clippy::trailing_empty_array)]
2+
#![allow(clippy::repr_packed_without_abi)]
23

34
// Do lint:
45

0 commit comments

Comments
 (0)